LLNL / serac

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

adding domain argument to solid/thermal load setters #1075

Closed kswartz92 closed 7 months ago

kswartz92 commented 7 months ago

This PR adds a domain argument to the load setters in the solid mechanics and thermal physics modules. It is a breaking change, although I suspect most of the time users will not want to apply loads to the entire domain (perhaps this isn't true for body loads). It doesn't seem too bad to provide the entire mesh using the EntireDomain or EntireBoundary helper functions, although it does add another required argument. Curious to hear everyone's thoughts on this suggested interface change

codecov-commenter commented 7 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (253d1c8) 88.76% compared to head (1210fe0) 88.79%.

Files Patch % Lines
src/serac/physics/tests/solid_finite_diff.cpp 94.11% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1075 +/- ## =========================================== + Coverage 88.76% 88.79% +0.02% =========================================== Files 154 154 Lines 12480 12491 +11 =========================================== + Hits 11078 11091 +13 + Misses 1402 1400 -2 ```

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

jamiebramwell commented 7 months ago

One way to avoid the interface breaking change is to have the Domain arguments be std::optional with a default null value. If the argument is not supplied, a domain from the entire mesh can be constructed. Any thoughts @samuelpmishLLNL ?

jamiebramwell commented 7 months ago

It could also default construct an empty domain to avoid the use of std::optional.

samuelpmishLLNL commented 7 months ago

Right now, the Domain class doesn't have a default constructor, since it has a const mfem::Mesh & member that must be initialized. But any of the following seem like reasonable approaches to me:

btalamini commented 7 months ago

Actually, it's probably a good idea to add this to the material calls as well so we can have multiple functional forms in a single simulation.

We could do that now and it would work for elastic material models. For inelastic models, we need extra logic to allocate the correct number of internal variables per material and to stride through those correctly during assembly. Can we get there in an incremental fashion without holding up this PR?

jamiebramwell commented 7 months ago

Actually, it's probably a good idea to add this to the material calls as well so we can have multiple functional forms in a single simulation.

We could do that now and it would work for elastic material models. For inelastic models, we need extra logic to allocate the correct number of internal variables per material and to stride through those correctly during assembly. Can we get there in an incremental fashion without holding up this PR?

Good point. I missed the part about materials with state needing some extra work. I'll go ahead and approve this.