pmgbergen / porepy

Python Simulation Tool for Fractured and Deformable Porous Media
GNU General Public License v3.0
251 stars 88 forks source link

Compute geometry of `bg` when `sd` is replaced #1217

Open jhabriel opened 2 months ago

jhabriel commented 2 months ago

Howdy!

I'm currently working on adaptive refinement and my first take on updating the mdg is to use the replace_subdomains_and_interfaces method. The method works as intended, but the boundary grid associated to the new_sd does not have its geometry computed.

This was a bit strange to me, since the geometry is almost immediately used to update the boundary conditions of a typical PorePy model. Is there a reason behind this design choice?

All the best!

IvarStefansson commented 2 months ago

Well, I guess it makes sense the boundary grids are not updated, given the method name. But I see no immediate reason why we couldn't rename it and include the boundary grid update. But the update might entail some additional mapping of stored values.

keileg commented 2 months ago

This is not by design. Most likely we forgot to update the functionality for grid replacement when the boundary grids were introduced. AFAIK, there are no tests where we first replace a grid and then actually use the grid for anything, so it makes sense this has been undetected until now.

jhabriel commented 2 months ago

@IvarStefansson: I believe the old-to-new boundary data is actually mapped. It is just that the geometry of the new boundary grid is not computed.

@keileg: I see. Yes, it does make sense.

Would you be interested in some unit tests for mdg.replace_subdomain_and_interfaces())? Something like replacing the grid and then check for expected geometry / projection operators to be in place?

keileg commented 2 months ago

Would you be interested in some unit tests for mdg.replace_subdomain_and_interfaces())? Something like replacing the grid and then check for expected geometry / projection operators to be in place?

We would be very grateful.

Where to put these is not immediately clear: There already are some tests that use refinement, but their focus on the update of the mortar grid, hence the placement in that particular file. Anyhow, if you are willing to write some tests, we will take care of matters such as location.

IvarStefansson commented 2 months ago

@IvarStefansson: I believe the old-to-new boundary data is actually mapped. It is just that the geometry of the new boundary grid is not computed.

I was actually thinking of mapping of stored variables, boundary conditions etc.