Skip to content

Conversation

@miaoyinb
Copy link
Contributor

closes #31759

@miaoyinb miaoyinb force-pushed the transitionXYZD branch 2 times, most recently from 82199b6 to a082057 Compare October 21, 2025 03:08
@moosebuild
Copy link
Contributor

moosebuild commented Oct 21, 2025

Job Documentation, step Docs: sync website on b7bd6c7 wanted to post the following:

View the site here

This comment will be updated on new commits.

@miaoyinb miaoyinb force-pushed the transitionXYZD branch 3 times, most recently from c16dc84 to abe61df Compare October 22, 2025 04:23
@moosebuild
Copy link
Contributor

moosebuild commented Oct 22, 2025

Job Coverage, step Generate coverage on b7bd6c7 wanted to post the following:

Framework coverage

2acb41 #31761 b7bd6c
Total Total +/- New
Rate 85.99% 85.99% +0.00% 100.00%
Hits 124177 124212 +35 136
Misses 20240 20240 - 0

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@miaoyinb miaoyinb marked this pull request as ready for review October 23, 2025 13:23
@GiudGiud GiudGiud self-assigned this Oct 23, 2025
@idaholab idaholab deleted a comment from moosebuild Oct 25, 2025
issues = '#31293'
issues = '#31293 #31759'
[simple_hex8]
type = Exodiff
Copy link
Contributor

Choose a reason for hiding this comment

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

what s the motivation for this?
now that the exodus files have been checked in (and their memory cost paid) it s as good a test as it can be for comparing meshes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The conversion of QUAD4 faces to TRI3 faces relies on global node ids. Distributed mesh mode gives inconsistent node indexing and thus inconsistent division of QUAD4 faces.

Copy link
Contributor

Choose a reason for hiding this comment

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

even with allow-renumbering = false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, the GMG creates different node ids.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should fix this.
is this PR awaited for analysis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it is not awaited.

Copy link
Contributor

Choose a reason for hiding this comment

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

@roystgnr how big of a lift is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is mostly @miaoyinb's code in MooseMeshElementConversionUtils, isn't it? libMesh all_tri() uses geometry to determine which diagonal to turn into an edge, mostly to get higher quality elements, but also to get an output independent of numbering. But that would also need refactoring (to work on ranges of elements rather than whole meshes) to use here; it's not a drop-in replacement either. It would have been easier to refactor and extend originally than to attempt to rewrite, but only hindsight is 20/20, and IMHO it would now be a real delay to fix it either way.

Copy link
Contributor

@GiudGiud GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

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

ok seems we have a path forward, which is to extend libMesh to allow a range of elements on that routine.

I think in the meantime we should revert the test changes, and set mesh_mode = replicated

Co-authored-by: Guillaume Giudicelli <[email protected]>
@moosebuild
Copy link
Contributor

Job Test, step Results summary on b7bd6c7 wanted to post the following:

Framework test summary

Compared against 2acb415 in job civet.inl.gov/job/3334968.

Removed tests

Test Time (s)
meshgenerators/boundary_element_conversion_generator.err_distributed SKIP

Added tests

Test Time (s)
meshgenerators/xyz_delaunay_generator.xyzdelaunay/surface_convert_and_stitch 1.12
meshgenerators/xyz_delaunay_generator.xyzdelaunay/stitching_combined 0.89

Run time changes

Modules test summary

Compared against 2acb415 in job civet.inl.gov/job/3334968.

Removed tests

Added tests

Run time changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some Improvements for XYZDelaunayGenerator

4 participants