moose
moose copied to clipboard
idaholab
side_discontinuous option for Exodus output
Reason
MOOSE has had support for SIDE_DISCONTINUOUS variables for a while, but no way to visualize them easily. This should fix that.
Design
By setting side_discontinuous=true in Exodus output, disconnected fictitious side elements are added to that output, on which SIDE_DISCONTINUOUS values are plotted, with the discontinuities preserved.
A couple new DGKernel subclasses, HFEMTestJump and HFEMTrialJump, let us test this using a simple HFEM problem, solved with SIDE_DISCONTINUOUS lagrange multiplier rather than with the non-fictitious side element option previously available.
Impact
Additional capabilities for Exodus.
DGKernels now "count" when our error checking tests to make sure that every variable in the system has an associated kernel.
Refs #20191. It won't close that, though; we still have more 3D element types to add support for.
There's a bit of copy-and-paste code I still need to refactor into a new base class for SideObject+InternalSideObject, but I'm out of time for today and I'd like to find out whether CI agrees with my workstation that this is passing tests.
Job Precheck on 0936a69 wanted to post the following:
Your code requires style changes.
A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:
curl -s https://mooseframework.inl.gov/docs/PRs/22324/clang_format/style.patch | git apply -v
Alternatively, with your repository up to date and in the top level of your repository:
git clang-format 17f873bd9936bc936dab9e8d3356d693377b7c3e
Job Documentation on 49c389f wanted to post the following:
View the site here
This comment will be updated on new commits.
Job Coverage on 49c389f wanted to post the following:
Framework coverage
| 67205b | #22324 49c389 | ||||
|---|---|---|---|---|---|
| Total | Total | +/- | New | ||
| Rate | 84.39% | 84.40% | +0.01% | 89.02% | |
| Hits | 80682 | 80833 | +151 | 146 | |
| Misses | 14923 | 14937 | +14 | 18 | |
Modules coverage
Coverage did not change
Full coverage reports
Reports
-
framework -
chemical_reactions -
combined -
contact -
electromagnetics -
external_petsc_solver -
fluid_properties -
fsi -
functional_expansion_tools -
geochemistry -
heat_conduction -
level_set -
misc -
navier_stokes -
peridynamics -
phase_field -
porous_flow -
ray_tracing -
rdg -
reactor -
richards -
scalar_transport -
solid_properties -
stochastic_tools -
tensor_mechanics -
thermal_hydraulics -
xfem
Warnings
-
frameworknew line coverage rate 89.02% is less than the suggested 90.0%
This comment will be updated on new commits.
I think those nonsparse failures are likely real failures
After half a day of trying to figure this out: they're worse than real failures. Real failures mean I wrote code that didn't do what I meant, and need to write code that does what I meant instead. These failures are failures of definition, where I didn't actually have a straightforward idea of what I meant.
DGKernel calculations appear to use "which element has the higher id" to decide which side of an element counts as the element vs the mere neighbor ... which would mean that as soon as we add a few processors to a distributed run and DistributedMesh helpfully reshuffles ids to give us contiguous id blocks on each rank, the definition of "element" vs "neighbor" inverts, and the definition of a Lagrange multiplier variable constraining directional jumps from element to neighbor also inverts, and exodiff starts whining at me about having rightfully noticed that -1/6 and 1/6 are two different numbers.
I guess the right thing to do here is something like a vertex_average()-sorting-dependent sign flip in the new HFEM kernels, to make the results depend on geometry without regard for numbering.
I bet the old HFEM kernels with the non-fictitious side elements have the same problem, but never trigger it because they don't run distributed...
I would contend that difference only by sign in a Lagrange multiplier is not a true failure at all. What I would do is create a custom comparison that doesn't exodiff based on the LMs ... but then again I guess the whole point of this PR is the exodus output of these LMs :laughing: Would it be possible to pass the LM field through an aux kernel that takes the absolute value of the LM and then only write out that field?
I guess the whole point of this PR is the exodus output of these LMs
Admittedly I do feel like I've gone down the rabbit hole here. I haven't bothered to push anything until it's all fixed, but as part of fixing those doc failures I've written a test case to add to the documentation of a base class I refactored to properly support the postprocessor I wrote for a different test case ...
But I do want this stuff to be useful too, and if I can get this working (my quick fix idea seems to have worked perfectly in 1D but not at all in 2D???) then making it easier for future kernels to be tested via exodiff will be a valuable thing to have too.
Plus I really want to understand what I'm still doing wrong.
Okay, I get it now. After making the orientation of lambda depend on geometry independent of numbering, I now get consistent lambda values on all side elements, and I was already getting consistent u values on all interiors ... but I'm also plotting u on those side elements. And in this test u is discontinuous. And I plot its value on a side exactly once, from whichever interior I happen to hit first when I'm plotting that side. So if renumbering means I hit interiors in different orders, then I can plot the same internal side from different elements in different partitionings, and it can take on different values.
Technically u is discontinuous, IMHO it would be fair to just plot it as 0 or NaN on sides ... but users might find it more useful to plot it as the average of the values between the two interiors, and that's pretty much what we already do when plotting discontinuous fields at shared nodes in other contexts, too.
Job Precheck on 1c15426 wanted to post the following:
Your code requires style changes.
A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:
curl -s https://mooseframework.inl.gov/docs/PRs/22324/clang_format/style.patch | git apply -v
Alternatively, with your repository up to date and in the top level of your repository:
git clang-format b4abe9f2057135c183fbc01a98ff639246b75c65
Is there a way to hit InternalSideUserObject::getFaceInfos?
I've been banging my head on this but I'm not sure how to do it. That code path is only hit for integration of FV variables, FV variables have to be constant, and with HFEM (or with DPG) you need higher order for interior variables. I guess I could drop the idea of involving a SIDE_HIERARCHIC variable in the test, and add a postprocessor subclass to an FVM solve?
Yea probably a new distinct test would be the easiest. Let me know if you'd like any help cooking one up
And of course now I need an issue tag to add to the test that I added to cover the framework feature that I added to cover the postprocessor that I added to test the kernels that I added to test the output I added to visualize the library feature I added to swallow the cat to catch the bird to catch the spider to catch the fly...
That was surprisingly straightforward to follow up to the cat :laughing: :grimacing: :cry: