Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix FSISPH and constant boundaries #308

Merged
merged 2 commits into from
Oct 14, 2024
Merged

bugfix FSISPH and constant boundaries #308

merged 2 commits into from
Oct 14, 2024

Conversation

jmpearl
Copy link
Collaborator

@jmpearl jmpearl commented Oct 8, 2024

Summary

  • the damaged pressure was being initialized at 2x. This would cause issues for problems w/ constant boundaries.

ToDo :

  • Annotate RELEASE_NOTES.md with notable changes.
  • Create LLNLSpheral PR pointing at this branch. (PR#)
  • LLNLSpheral PR has passed all tests.

@jmpearl jmpearl self-assigned this Oct 8, 2024
@jmpearl jmpearl requested a review from jmikeowen October 8, 2024 15:42
Copy link
Collaborator

@jmikeowen jmikeowen left a comment

Choose a reason for hiding this comment

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

Assuming the damaged pressure is always otherwise initialized this is fine. Another fix would have been to initialize here with
mDamagedPressure = this->pressure();
rather than adding it. Why was this done with += originally?

@jmpearl
Copy link
Collaborator Author

jmpearl commented Oct 9, 2024

That is a great question and i asked myself the same thing. I can't remember why I had it as plus equals originally. I think the damaged pressure should always be initialized now through the call to the pressure update policy. Do you see any edge cases where that might not be the case?

@jmikeowen
Copy link
Collaborator

Naw, I think you should be good as is. Still have to ask about having a test that would catch this error of course... :-)

@jmpearl
Copy link
Collaborator Author

jmpearl commented Oct 14, 2024

I'm going to merge this now just to get the change in and punt on the test to a later date

@jmpearl jmpearl merged commit 7ad743c into develop Oct 14, 2024
1 check passed
@jmpearl jmpearl deleted the bugfix/FSISPH branch October 14, 2024 20:07
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.

2 participants