Skip to content

Conversation

@paveltomin
Copy link
Collaborator

@paveltomin paveltomin commented Sep 19, 2024

should go after #3227

  • move calculateElementCenters for subregions earlier into ProblemManager::generateMeshLevel, so cell centers for surface elements are available when well perforation is processed
  • remove registerWrapper for elementCenter and elementVolume in EmbeddedSurfaceSubRegion, they already registered in the base class
  • modify getReservoirElementDimensions/getBoundingBox to support SurfaceElementSubRegion
  • add SurfaceElementSubRegion version of isPointInsideElement and helper functions isPointInPolygon2d/3d

(not super refined implementation but something to start working with)

@rrsettgast rrsettgast added the ci: run CUDA builds Allows to triggers (costly) CUDA jobs label Aug 21, 2025
@jhuang2601
Copy link
Contributor

Hello @rrsettgast and @castelletto1, could you please take a final review of this PR, which is expected for InSalah case @herve-gross

@rrsettgast
Copy link
Member

Hello @rrsettgast and @castelletto1, could you please take a final review of this PR, which is expected for InSalah case @herve-gross

@jhuang2601
I didn't review, but I approved it. I don't have coverage for my time on any GEOS tasks any longer.

@jhuang2601 jhuang2601 requested a review from bd713 October 3, 2025 18:36

// remove duplicates
std::unordered_set< Point3d, PointHash< Point3d >, PointsEqual< Point3d > >
unique_points( 0, PointHash< Point3d >(), PointsEqual< Point3d >( geomTol ));
Copy link
Contributor

Choose a reason for hiding this comment

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

In practice, I'm not sure the unordered_set will handle this sort of tolerance-based equality.
Two close coordinates might be seen equal within the tolerance, but still, I would expect both to generate two different hashes, meaning both will be seen as distinct, and kept in the list.

(point[1] <= std::max( p1[1], p2[1] )) &&
(point[0] <= std::max( p1[0], p2[0] )))
{
real64 const xIntersect = (point[1] - p1[1]) * (p2[0] - p1[0]) / (p2[1] - p1[1]) + p1[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you confirm the expected behavior/convention for a point on the boundary?
I have the impression that a point on a vertical left edge will be considered "outside", while a point on a right edge will be "inside". Same with a horizontal top edge (inside) vs. bottom (outside)

{
LvArray::tensorOps::add< 3 >( elementCenters[k], X[e2n( k, a )] );
localIndex const nA = e2n( k, a );
auto const [it, inserted] = uniqueNodes.insert( {X[nA][0], X[nA][1], X[nA][2]} );
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a small tolerance to better detect duplicated nodes?

@paveltomin paveltomin requested a review from OmarDuran as a code owner October 6, 2025 14:15
@paveltomin paveltomin added the ci: run integrated tests Allows to run the integrated tests in GEOS CI label Oct 6, 2025
@paveltomin paveltomin added flag: requires rebaseline Requires rebaseline branch in integratedTests and removed flag: ready for review flag: no rebaseline Does not require rebaseline labels Oct 7, 2025
@paveltomin paveltomin merged commit 0f7eab5 into develop Oct 7, 2025
25 of 26 checks passed
@paveltomin paveltomin deleted the pt/connect-well-to-2d-elem branch October 7, 2025 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: requires rebaseline Requires rebaseline branch in integratedTests type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants