Skip to content

Conversation

@RomainBaville
Copy link
Contributor

@RomainBaville RomainBaville commented Oct 31, 2025

This pr aims to:

  • Move GeosBlockMerge from geos-posp to geos-processing
  • Refactor GeosBlockMerge to be coherent with the rest of the code

The implementation of the tests for GeosBlockMerge will be make later with a clean of all the tests and the data. See more clean test

@RomainBaville RomainBaville self-assigned this Oct 31, 2025
@paloma-martinez paloma-martinez changed the title refactor: Move GeosBlockMerge from geos-prop to geos-processing refactor: Move GeosBlockMerge to geos-processing Nov 3, 2025
@RomainBaville RomainBaville added the test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI label Nov 4, 2025
Comment on lines 69 to 72
for all the composite blocks of the input mesh:
- Ranks are merged
- "Rock" attributes are renamed
- Volume mesh are convert to surface if needed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for all the composite blocks of the input mesh:
- Ranks are merged
- "Rock" attributes are renamed
- Volume mesh are convert to surface if needed
For each composite block of the input mesh:
- Ranks are merged.
- "Rock" attributes are renamed.
- Volume meshes are converted to surface if requested.

Comment on lines 157 to 162
surfaceMesh: vtkPolyData = self.convertBlockToSurface( volumeMesh )
assert surfaceMesh is not None, "Surface extraction from block failed."
surfaceMesh.ShallowCopy( computeNormals( surfaceMesh, logger=self.logger ) )
assert surfaceMesh is not None, "Normal calculation failed."
surfaceMesh.ShallowCopy( computeTangents( surfaceMesh, logger=self.logger ) )
assert surfaceMesh is not None, "Tangent calculation failed."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three assert can be handled in the except.

Co-authored-by: paloma-martinez <[email protected]>
Copy link
Contributor

@jafranc jafranc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good ! Looking forward for upcoming refactor PR, feel free to comment this one with their numbers if opened.

Comment on lines +105 to 110
PHASE_DENSITY_SUFFIX = "_density"
PHASE_MASS_DENSITY_SUFFIX = "_phaseMassDensity"
PHASE_VISCOSITY_SUFFIX = "_phaseViscosity"
PHASE_VISCOSITY_SUFFIX = "_viscosity"
PHASE_FRACTION_SUFFIX = "_phaseFraction"

# surface attribute transfer suffix
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would worth its own PR to set the strategy here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought that the names just changed, that why I just modified it, but I figure out that names were case dependent. In this pr I just change the names to be coherent with single phase flow simulation, in the pr #169 I add the names for multi phase flow simulation. An pr need to be done to check this name indeed.

Comment on lines +40 to +60
from geos.processing.post_processing.GeosBlockMerge import GeosBlockMerge
# Filter inputs.
inputMesh: vtkMultiBlockDataSet
# Optional inputs.
convertFaultToSurface: bool # Defaults to False
speHandler: bool # Defaults to False
# Instantiate the filter
mergeBlockFilter: GeosBlockMerge = GeosBlockMerge( inputMesh, convertFaultToSurface, speHandler )
# Set the handler of yours (only if speHandler is True).
yourHandler: logging.Handler
mergeBlockFilter.setLoggerHandler( yourHandler )
# Do calculations
mergeBlockFilter.applyFilter()
# Get the multiBlockDataSet with one dataSet per region
outputMesh: vtkMultiBlockDataSet = mergeBlockFilter.getOutput()
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not providing an example using GeosBlockExtractor as it sounds mandatory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not mandatory to use GeosBlockExtractor before using this filter. This filter can be use to merge any type of mesh as long as it is a multiBlockDataSet with composite block to process. If you don't use GeosBlockExtractor or a mesh from a GEOS simulation only the renaming of attributes can be skiped but the filter will merge the mesh.

@paloma-martinez paloma-martinez merged commit e752019 into main Nov 14, 2025
56 checks passed
@paloma-martinez paloma-martinez deleted the RomainBaville/refactor/MoveGeosBlockMerge branch November 14, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flag: ready for review test-geos-integration Triggers the testing of geosPythonPackages import and integration in GEOS CI type: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants