mesh-adaptation / goalie

Goal-oriented error estimation and mesh adaptation for finite element problems solved using Firedrake
Other
2 stars 0 forks source link

Parallel-safe mesh-to-mesh interpolation #128

Closed ddundo closed 6 months ago

ddundo commented 7 months ago

Closes #39.

Introduces parallel-compatible mesh-to-mesh interpolation (already wrapped up in firedrake as f_dest = Function(V_dest).interpolate(f_src) where two VertexOnlyMeshes are used as intermediaries).

ddundo commented 7 months ago

Hi @jwallwork23 and @stephankramer, after reading the whole day about VertexOnlyMeshes and petsc's star forest, I realised that the entire procedure has already been wrapped up super nicely within the usual interpolate and is in the main firedrake branch. So I am creating a super early draft pull request here to just demonstrate how simple it appears to be and to ask for a general strategy before doing more.

The biggest question is what to do with project - do we want to keep both options? I guess this PR should also address #123 and #44. We'd also need to make plotting parallel-compatible (could be addressed in #117). Anything else?

jwallwork23 commented 7 months ago

Thanks for this @ddundo. Hopefully you learnt something while looking into VertexOnlyMesh and PETSc star forest!

I noticed the test failures on this PR so I tried out the mesh-to-mesh interpolation functionality in Firedrake myself. It seems like there are still some teething problems. I tried interpolating between two UnitSquareMeshes and got DofNotDefinedErrors. Weirdly, these still persisted even when I tweaked the target mesh so that all of its DoFs lie within the source domain. I then tried a case where the target mesh is already fully embedded within the source mesh, but still got DofNotDefinedErrors for all DoFs in the target space.

Regarding your question, let's not try to fix too many things in one PR. I think we should have both interpolation and projection options available.

ddundo commented 7 months ago

Thanks @jwallwork23! Regarding test failures and your review comment, actually when I run the tests locally only the gray_scott demo fails because interpolate raises the error NotImplementedError: UFL expressions for mixed functions are not yet supported..

1) So yes, potentially I'll overload interpolate - I still have to test more 2) About the other errors, I was surprised to see them here since they pass locally. Could you please try running the tests locally as well?

jwallwork23 commented 7 months ago

Hm no I get the same errors running locally:

====================================== short test summary info ======================================
FAILED test_demos.py::test_demos[burgers] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[burgers_time_integrated] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[gray_scott] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[burgers-hessian] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[burgers2] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[burgers_ee] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
FAILED test_demos.py::test_demos[burgers_oo] - subprocess.CalledProcessError: Command '['/home/firedrake/firedrake/bin/python3', '/home/joe/sof...
================ 7 failed, 121 passed, 3 skipped, 123 warnings in 286.62s (0:04:46) =================
ddundo commented 7 months ago

Hmm thanks @jwallwork23, that's odd. I guess you are using the same docker image as the workflow here? Here is the results of my firedrake-status and the output of goalie test suite out.txt

---------------------------------------------------------------------------
|Package             |Branch                        |Revision  |Modified  |
---------------------------------------------------------------------------
|FInAT               |master                        |e3a869d   |False     |
|PyOP2               |master                        |e0a4d3a9  |False     |
|animate             |main                          |b39ff5c   |False     |
|concave_hull        |master                        |bdca55e   |False     |
|fiat                |master                        |dbc1c5d   |False     |
|firedrake           |master                        |63104ed4f |False     |
|goalie              |39_vom_parallel_interpolation |6faac5d   |False     |
|h5py                |firedrake                     |6b512e5e  |False     |
|icepack             |master                        |28eed36   |True      |
|libsupermesh        |master                        |84becef   |False     |
|loopy               |main                          |8158afdb  |False     |
|petsc               |jwallwork23/parmmg-rebased    |30d0933c23|False     |
|pyadjoint           |master                        |2c6614d   |False     |
|pytest-mpi          |main                          |8241bdc   |False     |
|tsfc                |master                        |90c20c5   |False     |
|ufl                 |master                        |054b0617  |False     |
---------------------------------------------------------------------------

I noticed the test failures on this PR so I tried out the mesh-to-mesh interpolation functionality in Firedrake myself. It seems like there are still some teething problems. I tried interpolating between two UnitSquareMeshes and got DofNotDefinedErrors. Weirdly, these still persisted even when I tweaked the target mesh so that all of its DoFs lie within the source domain. I then tried a case where the target mesh is already fully embedded within the source mesh, but still got DofNotDefinedErrors for all DoFs in the target space.

Could you please send me this script?

jwallwork23 commented 7 months ago

Yeah, I'm using the docker image locally rather than installing.

Attached scripts here. interp.tar.gz

ddundo commented 7 months ago

Thanks @jwallwork23, these both work for me locally. Could you please check if the docker image needs to be updated? I'm clueless for docker sadly :) but no rush! Sorry for the spam today

jwallwork23 commented 7 months ago

Thanks @jwallwork23, these both work for me locally. Could you please check if the docker image needs to be updated? I'm clueless for docker sadly :) but no rush! Sorry for the spam today

Yeah annoyingly the weekly automated Docker build hasn't worked recently for some reason (see https://github.com/pyroteus/animate/issues/79). I'll rebuild it now, push to DockerHub and then rerun the tests here.

ddundo commented 7 months ago

Yeah annoyingly the weekly automated Docker build hasn't worked recently for some reason (see pyroteus/animate#79). I'll rebuild it now, push to DockerHub and then rerun the tests here.

Thanks for doing that! Unfortunately there are still some failures here but everything passes on my local install.

jwallwork23 commented 7 months ago

Thanks for doing that! Unfortunately there are still some failures here but everything passes on my local install.

No worries. I notice that Gray-Scott is now fixed and all the errors are with Burgers. I wonder if the problem is with the demo...

ddundo commented 7 months ago

No worries. I notice that Gray-Scott is now fixed and all the errors are with Burgers. I wonder if the problem is with the demo...

Well let me get docker running locally and investigate - I don't want to waste your time (and I need to finally give docker a go anyway :) ). I will ping you with updates/questions. Thanks for the help!

ddundo commented 6 months ago

No rush at all with this, but it's ready for review when you have time please :) I just wanted to get it out of the way.

A few comments:

  1. I assigned interpolate/project to MeshSeq.transfer but there is already an existing function called transfer in indicate_errors. I don't think it's confusing but feedback is welcome
  2. The methodology in interpolation._transfer_adjoint is unfamiliar to me so please check that it and the docstring make sense
ddundo commented 6 months ago

Thanks again @jwallwork23, ready for a re-review :)

ddundo commented 6 months ago

@acse-ej321 Just pinging you here to make you aware that interpolation is now used as the main transfer method :) so you might need to pass transfer_method="project" to MeshSeq in your code if you want to go back to projection