GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
223 stars 89 forks source link

meshBody(0) and meshLevel(0) are hardcoded everywhere. #951

Closed joshua-white closed 2 years ago

joshua-white commented 4 years ago

We started with ambitions of supporting multiBody and multiLevel grids. If you type:

grep -r "getMeshLevel( 0 )" src/

you will see that meshBody(0) and meshLevel(0) are now hardcoded literally everywhere. Do we continue to support this feature if no one is using it? Or perhaps provide a shortcut accessor for 99% of the code that could care less about more complex grids, and leave this as an advanced feature for specialized solvers? The latter may be easiest.

AntoineMazuyer commented 4 years ago

If I remember well, we had discussion to maybe get rid of the mesh body. I don't know for the mesh levels.

But I agree a good medium term solution can be to have an easy shortcut accessor to the only defined mesh level.

joshua-white commented 4 years ago

I do see value in keeping the meshLevels, but much less in keeping meshBody. That is just my opinion though. We do want to support geometric multigrid methods in the future.

Just throwing out ideas, but what if we add the ability to set an "active" mesh level outside of the physics solvers:

domain->setActiveMeshLevel( level )

Most physics solvers could then just call

MeshLevel const & meshLevel = domain->getActiveMeshLevel()

We therefore remove the level selection logic from single-level physics solvers. A multilevel scheme can still make direct calls to domain->getMeshLevel( level ).

AntoineMazuyer commented 4 years ago

Seems a very reasonable solution to me

rrsettgast commented 4 years ago

The MeshBody can be removed. It is a placeholder to remind me of the the manner in which the GEOSDYN-L guys treat their granular material modeling. For example it is easier for them to model each grain as a MeshBody s.t. they may change how they model that specific body if required. i.e. refinement of the whole body. However this requires a contact enforcement method above the MeshBody concept, and a MeshBody based partitioning strategy. We can always modify for addition later.

I would rather stay away from the domain->setActiveMeshLevel(level) strategy. My experience with that in another large code I worked on was not positive. It would be preferable to pass in an additional parameter next to domain to indicate the mesh level that should be processed. This is a little more amenable to asynchronous scheduling....if we ever get there.

joshua-white commented 4 years ago

Okay, I will start on this today. @rrsettgast, based on your comments, we have a few options. Just let me know your preferences.

Regarding meshBody:

Second, I was hoping we could avoid hardcoding meshLevel 0 in the physics solvers. The options are:

Other ideas are welcome.

I'm bringing this up because I would also like to add internal mesh refinement capabilities soon, and I would probably do this by creating meshLevel 1 from 0, etc. I could always swap ordering, though, so that the fine mesh is always at 0.

rrsettgast commented 4 years ago

Okay, I will start on this today. @rrsettgast, based on your comments, we have a few options. Just let me know your preferences.

Regarding meshBody:

  • Completely remove it
  • Leave it as is, but just add a shortcut accessor at domain level, i.e. domain->getMeshLevel( level ) that assumes you want body0. I don't think there is any overhead of having the body concept, but I just want to hide it when 99% of solvers will never use it.

Completely Remove it.

Second, I was hoping we could avoid hardcoding meshLevel 0 in the physics solvers. The options are:

  • Don't worry about this till later. A bunch of meshLevel( 0 ) calls is not so bad, as it is intuitive what this means. In the documentation we make it clear that most solvers are single-level and always work on 0, while future solvers will support multi-level.
  • Add something like m_targetMeshLevel to SolverBase, and allow the user to set this in the XML. It would default to 0. We would call domain->getMeshLevel( m_targetMeshLevel ) in the derived solvers.

We need to figure out how to interface with a multi-level solver. Would we want to be able to call an "update" on a single level? Do we want to batch all the levels together? (I don't think this is the case). Since we will be doing this in the context of a Multigrid or AMR scheme we will process each level individually. So then the option really is between including a m_targetMeshLevel in the solver, or adding a parameter to the interface function parameters lists. Keep in mind that the m_targetMeshLevel approach will restrict us from doing operations on different mesh levels in an asynchronous manner, which is why I like the function parameter list option better.

Perhaps this is it's own discussion separate from the MeshBody removal.

Other ideas are welcome.

I'm bringing this up because I would also like to add internal mesh refinement capabilities soon, and I would probably do this by creating meshLevel 1 from 0, etc. I could always swap ordering, though, so that the fine mesh is always at 0.

I think we agreed that level 0 would be the base level, and we would have finer and coarser levels. So some would be positive, some would be negative. I don't recall which was which. I think the answer is different depending on what perspective you are approaching it from. Whatever the case, it doesn't matter what we choose as long as we are consistent.

TotoGaz commented 2 years ago

@rrsettgast We may want to close https://github.com/GEOSX/GEOSX/pull/1682 when is merged, don't we?