feat: Add ExternalDataRepository #2957

untereiner · 2024-01-26T14:51:19Z

May close #2953

More explanations:

There below is the situation today:

Every child of MeshGeneratorBase has to fulfill its API even if not compatible
ExternalMeshGeneratorBase has to have a file as input

classDiagram
namespace GEOS {

  class Group 

 class WellGeneratorABC

 class WellGeneratorBase
  
  class MeshGeneratorBase {
    generateMesh( Group & parent, SpatialPartition & partition ): void 
    importFieldOnArray( Block block, ....) : void virtual
    freeResources(): virtual void 
    getVolumicFieldsMapping(): std::map< string, string > const &
    getSurfacicFieldsMapping(): std::map< string, string > const & 
  }

  class ExternalMeshGeneratorBase {
    m_filePath: Path 
    m_translate: R1Tensor
    m_scale: R1Tensor
    m_volumicFieldsToImport: array1d< string >
    m_volumicFieldsInGEOS: array1d< string >
    m_surfacicFieldsToImport: array1d< string > 
    m_surfacicFieldsInGEOS: array1d< string >
  }

  class InternalMeshGenerator 
}

Group <|-- MeshGeneratorBase
MeshGeneratorBase <|-- ExternalMeshGeneratorBase
MeshGeneratorBase <|-- InternalMeshGenerator
Group <|-- WellGeneratorABC 
WellGeneratorABC <|-- WellGeneratorBase

What I expect after this PR:

A new class (ExternalDataRepository) with almost no constraints on what it expects allows me to create a proper API for another module (example: RESQML)
A new class (VTKHierarchicalDataRepository) to add the capability to load complex meshes with semantics (example provided on singlePhaseFlow)
A new class (Region) to add the capability to load sets of cells (vtkDataSets) from a complex mesh data format (VTK or RESQML) and give it a id for internal purpose (aka regionAttribute for vtu files)

classDiagram
direction LR
namespace GEOS {

class Group

class ExternalDataRepository {
  virtual open(); void
}

class MeshGeneratorBase {
  generateMesh( Group & parent, SpatialPartition & partition ): void 
  importFieldOnArray( Block block, ....) : void virtual
  freeResources(): virtual void 
  getVolumicFieldsMapping(): std::map< string, string > const &
  getSurfacicFieldsMapping(): std::map< string, string > const & 
}

class ExternalMeshGeneratorBase {  
  m_translate: R1Tensor
  m_scale: R1Tensor
  m_volumicFieldsToImport: array1d< string >
  m_volumicFieldsInGEOS: array1d< string >
  m_surfacicFieldsToImport: array1d< string > 
  m_surfacicFieldsInGEOS: array1d< string >
}

class VTKMeshGenerator {
  m_filePath: Path 
}

class InternalMeshGenerator 

class VTKHierarchicalDataRepository

class MeshComponentBase

class WellGeneratorBase

class Region

}

Group <|-- ExternalDataRepository
Group <|-- MeshGeneratorBase
MeshGeneratorBase <|-- ExternalMeshGeneratorBase
MeshGeneratorBase <|-- InternalMeshGenerator
ExternalMeshGeneratorBase <|-- VTKMeshGenerator
ExternalDataRepository <|-- VTKHierarchicalDataRepository
Group <|-- MeshComponentBase
MeshComponentBase <|-- WellGeneratorBase
MeshComponentBase <|-- Region


namespace GEOS-RESQML {

class EnergyMLDataObjectRepository {
common::DataObjectRepository * getData()
COMMON_NS::AbstractObject* getDataObject(string const & id)
COMMON_NS::AbstractObject* getDataObjectByTitle(string const & name)

}


class EpcDocumentRepository

class RESQMLMesh 
}

ExternalDataRepository <|-- EnergyMLDataObjectRepository
EnergyMLDataObjectRepository <|-- EpcDocumentRepository
MeshGeneratorBase <|-- RESQMLMesh

codecov · 2024-01-26T21:20:03Z

Codecov Report

Attention: Patch coverage is 34.70588% with 111 lines in your changes missing coverage. Please review.

Project coverage is 57.17%. Comparing base (b23c80e) to head (ebc60b9).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...oreComponents/mesh/generators/VTKMeshGenerator.cpp	28.84%	37 Missing ⚠️
...ents/mesh/generators/VTKHierarchicalDataSource.cpp	0.00%	28 Missing ⚠️
src/coreComponents/mesh/ExternalDataSourceBase.cpp	17.64%	14 Missing ⚠️
.../coreComponents/mesh/ExternalDataSourceManager.cpp	33.33%	14 Missing ⚠️
src/coreComponents/mesh/generators/Region.cpp	0.00%	12 Missing ⚠️
src/coreComponents/mesh/generators/Region.hpp	33.33%	2 Missing ⚠️
...ents/mesh/generators/VTKHierarchicalDataSource.hpp	33.33%	2 Missing ⚠️
...reComponents/mesh/generators/MeshGeneratorBase.cpp	66.66%	1 Missing ⚠️
...oreComponents/mesh/generators/VTKMeshGenerator.hpp	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2957      +/-   ##
===========================================
- Coverage    57.22%   57.17%   -0.05%     
===========================================
  Files         1143     1149       +6     
  Lines        98913    99044     +131     
===========================================
+ Hits         56600    56628      +28     
- Misses       42313    42416     +103

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

paveltomin · 2024-06-07T19:22:49Z

hi @untereiner
i am not sure how adding MeshBase layer helps here, most probably i am missing soemthing
could you please add some description explaining that?
thank you

untereiner · 2024-06-07T19:32:11Z

Hi @paveltomin,
I can link to the RESQML plugin. Adding this layer helps me creating a child node in Mesh by subclassing something more abstract that MeshGeneratoreBase

The entry point of the RESQML C++ library is a repository of 3D objects that I want to put in the deck.

I can also link an example deck if needed.

Also, VTK has file formats storing composite data (grids and surfaces) with complex relationships ( see here). I claim that these files should be considered as « data repositories » to extract from them what’s needed where it’s needed. So, this MeshBase layer could also benefit to them

paveltomin · 2024-06-07T20:53:38Z

Hi @paveltomin, I can link to the RESQML plugin. Adding this layer helps me creating a child node in Mesh by subclassing something more abstract that MeshGeneratoreBase

The entry point of the RESQML C++ library is a repository of 3D objects that I want to put in the deck.

I can also link an example deck if needed.

Also, VTK has file formats storing composite data (grids and surfaces) with complex relationships ( see here). I claim that these files should be considered as « data repositories » to extract from them what’s needed where it’s needed. So, this MeshBase layer could also benefit to them

thanks for the explanations!

TotoGaz

Given the 2 PRs (#3068 and #3063) that are deeply revamping this area of the code, this is unfortunately almost granted that this work (and all the work attached relying on it) will become obsolete quite soon.

TotoGaz · 2024-06-07T21:50:01Z

src/coreComponents/mesh/generators/MeshGeneratorBase.hpp

@@ -41,7 +38,7 @@ class CellBlockManager;
 *  @brief The MeshGeneratorBase class provides an abstract base class implementation for different mesh types.
 *	   The MeshGeneratorBase is the Group specialization for different type of mesh handling.
 */
-class MeshGeneratorBase : public dataRepository::Group
+class MeshGeneratorBase : public MeshBase


Doing so, you make the (yet to become) generators module depend on mesh while we have been willing to de the exact opposite: generators would become a "somehow" independent "source" of meshing informations.

I can rename it GeneratorBase. But that’s not the path you are drawing. I would be happy to revamp my work in a sibling modèle of the generator one called « data repository »

TotoGaz · 2024-06-07T21:53:25Z

src/coreComponents/mesh/MeshManager.cpp

@@ -43,15 +43,15 @@ MeshManager::~MeshManager()
 Group * MeshManager::createChild( string const & childKey, string const & childName )
 {
  GEOS_LOG_RANK_0( "Adding Mesh: " << childKey << ", " << childName );
-  std::unique_ptr< MeshGeneratorBase > solver = MeshGeneratorBase::CatalogInterface::factory( childKey, childName, this );
-  return &this->registerGroup< MeshGeneratorBase >( childName, std::move( solver ) );
+  std::unique_ptr< MeshBase > mesh = MeshBase::CatalogInterface::factory( childKey, childName, this );


Now MeshManager depends on MeshBase and MeshGeneratorBase.
Couldn't we have one single concept and use it? I'd say that would make the things clearer.

Is there an answer to this question? To me, if MeshBase and MeshGeneratorBase have different responsibilities and MeshGeneratorBase is intended to generate something derived from MeshBase, then I don't see a problem....but perhaps I don't understand.

MelReyCG · 2024-08-22T14:12:42Z

@untereiner does it modify this graph in the docs ?

untereiner · 2024-08-22T21:00:33Z

@MelReyCG it adds a level in the class hierarchy, so yes. The PRs of Thomas are modifying these files. I don’t know how to move this forward.

untereiner · 2024-09-03T14:27:12Z

Hi @rrsettgast, I put diagrams to explain a bit more what this PR is expected to improve

rrsettgast

Hi @untereiner .
Would it be better to rename the proposed MeshBase to MeshGeneratorBase, and rename the current MeshGeneratorBase to something more specific to its use case?

src/coreComponents/fileIO/vtk/VTKPolyDataWriterInterface.hpp

rrsettgast · 2024-09-03T17:27:50Z

src/coreComponents/mesh/MeshBase.hpp

+ *  @brief The MeshBase class provides an abstract base class implementation for different mesh types.
+ *	   The MeshBase is the Group specialization for different type of mesh handling.
+ */
+class MeshBase : public dataRepository::Group


So MeshBase is currently just to hold the catalog definition/instantiation? Is there plans for specifying a pure virtual interface here?

For now no. It is to avoid implementing the API of MeshGeneratorBase

rrsettgast · 2024-09-03T17:30:10Z

src/coreComponents/mesh/MeshManager.cpp

@@ -43,15 +43,15 @@ MeshManager::~MeshManager()
 Group * MeshManager::createChild( string const & childKey, string const & childName )
 {
  GEOS_LOG_RANK_0( "Adding Mesh: " << childKey << ", " << childName );
-  std::unique_ptr< MeshGeneratorBase > solver = MeshGeneratorBase::CatalogInterface::factory( childKey, childName, this );
-  return &this->registerGroup< MeshGeneratorBase >( childName, std::move( solver ) );
+  std::unique_ptr< MeshBase > mesh = MeshBase::CatalogInterface::factory( childKey, childName, this );


Is there an answer to this question? To me, if MeshBase and MeshGeneratorBase have different responsibilities and MeshGeneratorBase is intended to generate something derived from MeshBase, then I don't see a problem....but perhaps I don't understand.

untereiner · 2024-09-03T19:48:43Z

Hi @untereiner . Would it be better to rename the proposed MeshBase to MeshGeneratorBase, and rename the current MeshGeneratorBase to something more specific to its use case?

I am open to suggestions !
I don’t have any good idea since MeshGeneratorBase is really ment to be a parent of a specific family of generators.

paveltomin · 2024-11-19T20:22:58Z

@wrtobin could you please have another look?

…/meshbase

src/coreComponents/mesh/generators/Region.cpp

paveltomin · 2024-11-27T16:16:06Z

src/coreComponents/mesh/generators/Region.hpp

+ * This class has dataRepository::Group capabilities to allow for XML input.
+ *
+ */
+class Region : public MeshComponentBase


@rrsettgast @CusiniM is ok to call it Region?

Region seems like it is too generic? How does this "Region" differ from the other "Region"?

…-DEV/GEOS into feature/untereiner/meshbase

untereiner self-assigned this Jan 26, 2024

untereiner marked this pull request as draft January 26, 2024 14:51

untereiner requested a review from TotoGaz January 26, 2024 14:51

untereiner marked this pull request as ready for review January 26, 2024 20:39

untereiner mentioned this pull request Jan 27, 2024

Add a VTK well import #2551

Merged

untereiner force-pushed the feature/untereiner/meshbase branch from 9ed51e9 to 7d92c8a Compare June 3, 2024 08:00

untereiner added ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: ready for review ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Jun 3, 2024

untereiner requested review from ryar9534 and paveltomin June 3, 2024 08:09

paveltomin approved these changes Jun 7, 2024

View reviewed changes

TotoGaz reviewed Jun 7, 2024

View reviewed changes

rrsettgast self-requested a review August 28, 2024 14:44

untereiner force-pushed the feature/untereiner/meshbase branch from d3405a7 to 6f18a98 Compare September 3, 2024 10:01

untereiner requested review from wrtobin, CusiniM and MelReyCG as code owners September 3, 2024 10:01

untereiner changed the title ~~Add MeshBase~~ feat: Add MeshBase Sep 3, 2024

rrsettgast reviewed Sep 3, 2024

View reviewed changes

untereiner changed the title ~~feat: Add MeshBase~~ feat: Add ExternalDataRepository Sep 4, 2024

untereiner requested a review from corbett5 as a code owner September 4, 2024 13:12

paveltomin removed request for ryar9534, klevzoff, francoishamon, dkachuma, matteofrigo5, joshua-white, cssherman and jhuang2601 November 19, 2024 19:52

wrtobin approved these changes Nov 19, 2024

View reviewed changes

MelReyCG and others added 7 commits November 20, 2024 09:37

Merge branch 'develop' into feature/untereiner/meshbase

726417d

Merge branch 'develop' into feature/untereiner/meshbase

2be6e57

Merge remote-tracking branch 'origin/develop' into feature/untereiner…

8b6b325

…/meshbase

missing const

ce78f90

build fix

493bed2

Merge branch 'develop' into feature/untereiner/meshbase

4dcdba1

Update VTKPolyDataWriterInterface.cpp

00a5616

paveltomin reviewed Nov 27, 2024

View reviewed changes

src/coreComponents/mesh/generators/Region.cpp Outdated Show resolved Hide resolved

Update VTKMeshGenerator.cpp

e116f1b

paveltomin reviewed Nov 27, 2024

View reviewed changes

Pavel Tomin and others added 8 commits November 27, 2024 10:18

revert cosmetic

75dae46

remove unused

f1e8e29

bug fix

ff99ac0

Update .integrated_tests.yaml

a4ec2d4

Update BASELINE_NOTES.md

f4cb9c0

Update Region.cpp

71a6e51

revert

9b5a132

Merge branch 'feature/untereiner/meshbase' of https://github.com/GEOS…

ebc60b9

…-DEV/GEOS into feature/untereiner/meshbase

paveltomin merged commit 89a6066 into develop Nov 28, 2024
23 of 25 checks passed

paveltomin deleted the feature/untereiner/meshbase branch November 28, 2024 00:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add ExternalDataRepository #2957

feat: Add ExternalDataRepository #2957

untereiner commented Jan 26, 2024 •

edited

Loading

codecov bot commented Jan 26, 2024 •

edited

Loading

paveltomin commented Jun 7, 2024

untereiner commented Jun 7, 2024 •

edited

Loading

paveltomin commented Jun 7, 2024

TotoGaz left a comment

TotoGaz Jun 7, 2024

untereiner Jun 7, 2024

TotoGaz Jun 7, 2024

rrsettgast Sep 3, 2024

MelReyCG commented Aug 22, 2024 •

edited

Loading

untereiner commented Aug 22, 2024 •

edited

Loading

untereiner commented Sep 3, 2024

rrsettgast left a comment

rrsettgast Sep 3, 2024

untereiner Sep 3, 2024

rrsettgast Sep 3, 2024

untereiner commented Sep 3, 2024

paveltomin commented Nov 19, 2024

paveltomin Nov 27, 2024

rrsettgast Nov 27, 2024

feat: Add ExternalDataRepository #2957

feat: Add ExternalDataRepository #2957

Conversation

untereiner commented Jan 26, 2024 • edited Loading

codecov bot commented Jan 26, 2024 • edited Loading

Codecov Report

paveltomin commented Jun 7, 2024

untereiner commented Jun 7, 2024 • edited Loading

paveltomin commented Jun 7, 2024

TotoGaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MelReyCG commented Aug 22, 2024 • edited Loading

untereiner commented Aug 22, 2024 • edited Loading

untereiner commented Sep 3, 2024

rrsettgast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

untereiner commented Sep 3, 2024

paveltomin commented Nov 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

untereiner commented Jan 26, 2024 •

edited

Loading

codecov bot commented Jan 26, 2024 •

edited

Loading

untereiner commented Jun 7, 2024 •

edited

Loading

MelReyCG commented Aug 22, 2024 •

edited

Loading

untereiner commented Aug 22, 2024 •

edited

Loading