Skip to content

Add a side hierarchic nodal soln implementation#4494

Open
lindsayad wants to merge 1 commit into
libMesh:develfrom
lindsayad:side-hierarchic-nodal-value
Open

Add a side hierarchic nodal soln implementation#4494
lindsayad wants to merge 1 commit into
libMesh:develfrom
lindsayad:side-hierarchic-nodal-value

Conversation

@lindsayad

Copy link
Copy Markdown
Member

Given the previous comment here (which I've temporarily kept), I anticipate some debate on this, although I think the implementation is defensible given what we already do for projecting discontinuous solutions onto nodes for other output cases

Co-authored-by: Codex <codex@openai.com>
@lindsayad

Copy link
Copy Markdown
Member Author

If we agree on this implementation, I'd probably go and temporarily skip the affected MOOSE tests

@lindsayad

lindsayad commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

This is how side_hiearchic_out.e in MOOSE looks like on QUAD9 with this PR visualized via paraview. My current choice to not encode a value for element interior nodes is obvious. What do people think? I'll note that previously this gold file just had all zeroes for the aux_side_var and didn't even have any representation of side_var due to some quirks in the MOOSE output system. I'm a little surprised that @roystgnr decided this Exodiff test fulfilled this stated requirement, but I've certainly done worse in my time I'm sure.

The system shall initialize SIDE_HIERARCHIC variables as instructed

With this PR: CONSTANT SIDE_HIERARCHIC side_var

Screenshot 2026-07-08 at 1 31 08 PM

With this PR: FIRST SIDE_HIERARCHIC aux_side_var

Screenshot 2026-07-08 at 1 31 22 PM

@moosebuild

Copy link
Copy Markdown

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

Coverage

125e14 #4494 f68ea3
Total Total +/- New
Rate 65.65% 65.65% +0.00% 100.00%
Hits 78738 78751 +13 15
Misses 41207 41207 - 0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@roystgnr

roystgnr commented Jul 13, 2026

Copy link
Copy Markdown
Member

Given the previous comment here (which I've temporarily kept), I anticipate some debate on this

There's definitely a lot of room for debate here, and a whole spectrum of possible positions to take from "Roy was being too lazy" on the one end through "Roy was just being thoughtless" on the other, but I'm not sure I want to actually find out which one of those positions wins, and I am sure I like your solution here better.

@roystgnr roystgnr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's skip the affected tests and run CI again to be paranoid before merging; I just finally fixed that stupid openmp bug and I'm already getting used to seeing everything green again.

lindsayad added a commit to lindsayad/moose that referenced this pull request Jul 13, 2026
lindsayad added a commit to lindsayad/moose that referenced this pull request Jul 20, 2026
lindsayad added a commit to lindsayad/moose that referenced this pull request Jul 20, 2026
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.

3 participants