GEOS icon indicating copy to clipboard operation
GEOS copied to clipboard

Improve separation between business and geometry in mesh generators

Open TotoGaz opened this issue 2 years ago • 1 comments

Describe the issue

Currently, the mesh generation tools depend on business domain, for example by looping over the constitutive relations of the sub regions when importing the fields. Fixing the blurred lines will help future developments and validations.

Proposed cleanup

In MeshGeneratorBase (and especially in its implementations):

For a first PR (Done in https://github.com/GEOSX/GEOSX/pull/2212).

  • [x] Split importFields into business and a geometry part.

As a second step, split generateMesh into business and geometry part (Done in https://github.com/GEOSX/GEOS/pull/2272).

  • [x] Keeping the CellBlockManager* in the mesh/generator folder, and the DomainPartition, MeshBody in the business part. 

The refactoring for VTKMeshGenerator and InternalMeshGenerator looks quite identical and within close reach.


  • [x] The work for InternalWellGenerator may be a tad more challenging because WellElementRegion are directly created in the Generator, without the help of any CellBlock or FaceBlock like we do for the other types of (sub) regions. To be studied with care.

As a last architecture potential refactoring.

  • [ ] Challenge the CellBlocABC::getExternalProperties feature. Is it relevant w.r.t. MeshGeneratorBase::importFields?

On a more C++ technical point of view for a last PR (and during the continuous refactoring process).

  • [ ] Be sure that all the remaining business includes (core + tests) are removed.
  • [ ] Split the generator folder into its own cmake target, with the appropriate dependencies. Can we only expose the *ABC classes and a few utility classes?
  • [ ] Remove the cmake dependencies (core + tests) .
  • [ ] See what we can do with the public/private/interface feature proposed by cmake.
  • [ ] Search for the Region word in the generators folder to check if its the appropriate wording.
  • [ ] Update the unit tests accordingly.

TotoGaz avatar Dec 09 '22 23:12 TotoGaz

Particle manager (from PR https://github.com/GEOS-DEV/GEOS/pull/2767) puts back the dependency to SpatialPartition, MeshBody, MeshLevel, ParticleRegion...

@XL64 and myself had spent some time to remove this connection to the business domain, in order to let the module become a purely geometrical “module”.

My comment in the review https://github.com/GEOS-DEV/GEOS/pull/2767#discussion_r1367568662 asked to remove the link to business domain...

The PR was merged anyhow.

I have no more time to invest into any more cleaning. Therefore I unassign myself from this issue; I will not be working on it anymore.

TotoGaz avatar Mar 20 '24 01:03 TotoGaz