Some assembly refactors #26269
Some assembly refactors #26269
Conversation
Also make a comment about the incorrectness of JxW neighbor
We are reiniting with only an elem and no side so there is no normal information to get
lindsayad
force-pushed
the
assembly-refactors-25666
branch
2 times, most recently
from
12f8844
to
9a5907f
Compare
|
Job Documentation on 7d80d46 wanted to post the following: View the site here This comment will be updated on new commits. |
And consequently remove our uses of that bad computation in the repo
This reverts commit 87914fe.
This reverts commit 15323b6.
…aceRef This method is used heavily by mortar and we don't call the set neighbor subdomain ID API from there, and I don't think we need to
We are calling through to reinit methods with a lower-d elem with reference points from the higher-d elem. This is just plain wrong
This reverts commit 530ba88.
This reverts commit cedc170.
lindsayad
force-pushed
the
assembly-refactors-25666
branch
from
264204d
to
414ee46
Compare
|
@rui-hu @lingzou @lingzou-anl the tests that are exodiffing were previously using an incorrect calculation of JxW. Could someone check and see what you think of the new results? |
|
Job Coverage on 7d80d46 wanted to post the following: Framework coverage
Modules coverageCoverage did not change Full coverage reportsReports
This comment will be updated on new commits. |
||||||||||||||||||||||||||
|
@roystgnr I'd most like your review on 7d80d46. I think I would likely add a sentence to the assertion that says "you really should just be integrating with a single JxW, so for DG you should be using the JxW from the element side" (with "element" being MOOSE jargon for "the governing element for an across-face element pair") |
|
Let me know if you need a review |
|
I do need one 😄 |
|
@lindsayad it is great that @loganharbour can help with the review. this MR is a bit beyond my capabilities. |
|
It would be good for a SAM dev to consider the SAM failures though and whether what you are now getting for output files reflect better solutions |
|
This PR fixes issue #26328 |
@lindsayad I am trying to understand what needs to be done on SAM side, like what has been changed in this MR, and what are needed in SAM; or maybe there is nothing needed in SAM, but just to make sure the failed cases should be updated and to accept the changes. |
|
@lindsayad It seems to me that this MR may have introduced a bug somewhere. Please refer to one of the failed SAM test case: tests/components/SurfaceCoupling/GapHT.i Note that this problem has analytical solutions (please see the test head of that problem). This MR produced results very far away from the analytical solutions, so this makes me think maybe
unless I was completely wrong about the analytical solutions. |
|
It did not introduce a bug. It fixed a bug. I think that previously the JxW returned by the |
|
If this caused a diff from an analytical or reference solution, then it seems like maybe SAM was working around the bug |
|
On line 81 of |
Work done in support of issue #25666 and its PR #26073