Merge main into release#239
Open
stephankramer wants to merge 19 commits into
Open
Conversation
This seems to fix compilation errors that currently break the tests.
See mesh-adaptation/docs#156. This PR adds a separate test suite workflow for the new `release` branch. That test suite not only runs on Animate's `release` branch, but also with the `release` version of Firedrake.
Closes #225. Moves `to_petsc_local_numbering` to Animate and adds a basic unit test.
This changes to_petsc_local_numbering to take in a local vector (i.e. a
sequential vector that includes halo DOFs) and returns a local vector in
the "natural ordering" of the dmplex topological points. This is what
the dmplex parmmg interface expects from the metric input vector. Should
not change anything in serial.
Appears to fix the warnings about indefinite metric in parmmg tests -
which I think were caused by halo entries that were copied,
out-of-bounds, from the global vector we were providing previously
Closes #181
Partial addresses #215
The change to to_petsc_local_numbering and the added parallel tests, seems to make the stalls in the CI (#215) worse: happening most CI runs, rather than 50% of the time. Therefore changes in the (unrelated) serial checkpointing code have been included, which _seem_ to make the CI pass more reliably (not failing so far on ~8 runs) - but it is quite possible (likely?) that the underlying issue is not resolved.
Co-authored-by: Stephan Kramer <s.kramer@imperial.ac.uk>
Closes #234 Make the existing test fail on this bug by reordering, and add a test with multiple spatially varying metrics.
Firedrake PR firedrakeproject/firedrake#5002 removed get_petsc_dir from pyop2.utils in favour of the standalone petsctools package. The setup.py in this repo already uses petsctools, but quality.py was still importing from pyop2.utils, which now breaks at import time. Fixes g-adopt/g-adopt#492 Co-authored-by: Stephan Kramer <s.kramer@imperial.ac.uk>
Apparently topologies are now stored under their topology dm name. No idea why we're actually checking this....
Precision here hinges on the default solver parameters used in _compute_gradient_and_hessian which currently falls back on firedrake default for tolerance in outer iteration (rtol=1e-7). This appears to give an error ~ 8e-8, which is rounded to 1e-7 and thus fails the assert almost equal test with places=7. Setting a tighter tolerance indeed reduces the error as expected. Not entirely sure why the error has increased, but presumably was close previously as well but <5e-8 and thus passing the test.
…dir' Fixes to pass CI: * Use petsctools.get_petsc_dirs instead of pyop2.utils.get_petsc_dir * Change test to reflect new checkpoint layout * Loosen in tolerance in TestHessianMetric::test_bowl
Member
Author
|
Oh I guess I first need to fix the release mesh-adaptation container build.... |
Addresses #240 This is needed because we are instructing our users to build with `pip install --no-build-isolation` which tells pip to build in the current python env and ignores the build dependencies specified in pyproject.toml. Following the installation instructions for Firedrake release however, this environment may not have these build requirements installed already (notably Cython and a sufficiently new setuptools). Therefore we need to instruct users to run `pip install -r animate/requirements-build.txt` beforehand. TODO (after merge): * update wiki installation instructions * use the additional step in the CI
stephankramer
force-pushed
the
skramer/release-candidate
branch
from
628830c to
c515999
Compare
Member
Author
|
This appears to work now in the updated CI release container. @joewallwork : this is now current release, which contains two commits 68242c8 and 6c14106 that are not on main, followed by two revert commits, followed by a merging in of main. We could also simply reset release to main (i.e. discontinue release history) if you prefer. Moving forward, we should probably decide on a policy for the maintenance of release. If we want to maintain a continuous history for both release and main, we need to be quite careful how we handle both, i.e. no cherry-picking or rebasing of commits that are already in the other branch, a hot-fix needs to go in release first and then be merged into main such that release is always an ancestor of main, etc. Discontinuing release - i.e. resetting it whenever we want to sync it with main, probably after each firedrake release - makes life a lot easier but you have to warn people to not "git pull" on their local release checkout. FWIW g-adopt and probably Thetis are going for the discontinuous approach. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR undoes #230 completely because it consists of a number of commits that are already on main (see https://github.com/mesh-adaptation/animate/pull/230/commits) combined with a number of commits to undo changes on main (because the starting point for release wasn't actually compatible with then Firedrake release) - and these commits were all squashed into one. Meaning that after merging main into release, we will have the same changes twice in history, and also the undone changes would need to be re-done.
By adding an explicit revert commit, the combined effect of 68242c8 (the squashed commit of #230) and the revert commit is zero and thus we can cleanly merge main into release - at which point they are the same - which I think (hope!) is compatible with Firedrake release at the moment.
The only alternative I can see is if we discontinue the history of release - which is fine, we just need to warn people to delete their local release branch - so let me know if you prefer that.