geodynamics / pylith

PyLith is a finite element code for the solution of dynamic and quasi-static tectonic deformation problems.
Other
153 stars 97 forks source link

_mesh and _DM in field get out of sync #318

Closed rwalkerlewis closed 3 years ago

rwalkerlewis commented 3 years ago

These should be both referring to the same mesh object, however they do not.

baagaard-usgs commented 3 years ago

What is the context of this issue?

I would like to remove _mesh from pylith::topology::Field to avoid confusion. In general, Field::_mesh should only be used to get the coordinate system and maybe a few other attributes. It should not be used to get the DM.

knepley commented 3 years ago

When Robert creates the source label, he was using solution.mesh().dmMesh(), but the integratorDomain was using solution.dmMesh() to check for the label, and they did not match.

baagaard-usgs commented 3 years ago

Correct, he should use solution.dmMesh().

Mesh::_dm is not expected to match Field::_dm, so the "issue" is more bad design/interface for Field that still needs to be fixed, not an actual problem with the DMs being out of sync.

knepley commented 3 years ago

@baagaard-usgs There is a problem with MeshOps::createSubdomainMesh(). This is called from IntegratorDomain::initialize(). It passes .mesh(), but pulls out dmMesh() from that. This does not match the dmMesh() DM and things break. I see that you want to pass the coordinate system, but maybe we need to pass CoordSys and PetscDM instead?

baagaard-usgs commented 3 years ago

The underlying problem is that creating a label after the mesh is setup breaks some assumptions.

@knepley Is there any reason to make sure all labels are in the Problem _mesh object (would we ever want the Problem _mesh object for AMR, adjoints, etc) or should we rely on the PETSc DM object in the solution field to hold everything? If the latter, then we are essentially discarding the Problem _mesh object once we setup the solution field; this is fine but we need to agree.

knepley commented 3 years ago

@baagaard-usgs I do not see a reason to keep labels somewhere else. I see the point about a coordinate system, but I guess we can keep that in the _mesh spot instead.