search

LLNL / serac

Serac is a high order nonlinear thermomechanical simulation code
BSD 3-Clause "New" or "Revised" License
178 stars 31 forks source link

disambiguate by_attr implementation #1078

Closed samuelpmishLLNL closed 6 months ago

samuelpmishLLNL commented 6 months ago

fixes https://github.com/LLNL/serac/issues/1077, adds more tests to verify

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (03d7d72) 88.77% compared to head (c7335af) 88.80%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1078 +/- ## =========================================== + Coverage 88.77% 88.80% +0.02% =========================================== Files 154 154 Lines 12491 12505 +14 =========================================== + Hits 11089 11105 +16 + Misses 1402 1400 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kswartz92 commented 6 months ago

When using a 3D mesh, would ofBoundaryElements work exactly like ofFaces? And in 2D would ofBoundaryElements work exactly as ofEdges?

samuelpmishLLNL commented 6 months ago

When using a 3D mesh, would ofBoundaryElements work exactly like ofFaces? And in 2D would ofBoundaryElements work exactly as ofEdges?

They're not the same thing. For example, a 3D mesh has face elements on the interior and the boundary, but ofBoundaryFaces will not visit interior faces when evaluating the predicate.

kswartz92 commented 6 months ago

When using a 3D mesh, would ofBoundaryElements work exactly like ofFaces? And in 2D would ofBoundaryElements work exactly as ofEdges?

They're not the same thing. For example, a 3D mesh has face elements on the interior and the boundary, but ofBoundaryFaces will not visit interior faces when evaluating the predicate.

Got it, thanks! Should we maybe add ofBoundaryFaces to the test? I primarily use Domain for boundary attributes. Unless it is tested somewhere else in the repo I didn't notice?

samuelpmishLLNL commented 6 months ago

maybe the ofFaces and ofEdges should be removed in favor of ofInteriorFaces and ofBoundaryFaces. I wonder, is there a real use-case where one would want to include boundary and interior faces in the same domain?

kswartz92 commented 6 months ago

maybe the ofFaces and ofEdges should be removed in favor of ofInteriorFaces and ofBoundaryFaces. I wonder, is there a real use-case where one would want to include boundary and interior faces in the same domain?

I think it could be useful for solving physics on a partial domain. E.g. an interior material interface may become the domain boundary along with boundary faces.