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

Various robustness additions to the Davis EOS #374

Merged
merged 26 commits into from
May 14, 2024
Merged

Conversation

jhp-lanl
Copy link
Collaborator

@jhp-lanl jhp-lanl commented May 11, 2024

PR Summary

Three primary additions to the Davis Reactants and Products EOS to make them more robust:

  1. Added the robust::ratio function when dividing by density
  2. Added early returns for non-positive densities. Since the products EOS should exhibit ideal gas behavior at zero density, it makes sense to return zeros for the reference values.
  3. Maxed the density with zero in the reactants EOS. With the robust ratio this basically just returns a low density limit. I don't think there's an overflow issue, but I could be wrong.
  4. Added the correct minimum energy as a function of density

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • N/A Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file.
  • LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

  • Update the version in cmake.
  • Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

@jhp-lanl jhp-lanl requested review from Yurlungur, dholladay00 and chadmeyer and removed request for dholladay00 May 11, 2024 02:57
@jhp-lanl
Copy link
Collaborator Author

@chadmeyer please review these changes and see if you agree with my approach

singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

I defer to @chadmeyer 's comments. But other than what chad picked out I'm happy. Thanks for the fixes.

singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_davis.hpp Show resolved Hide resolved
@jhp-lanl
Copy link
Collaborator Author

@chadmeyer I think I addressed all your comments. I'll give things another scan and make sure I didn't miss anything else silly, but I think this is good now.

@jhp-lanl
Copy link
Collaborator Author

FYI - I added a few more robustness additions. Mainly I added an error to the DensityEnergyFromPressureTemperature() functions if the resultant density is negative (adding a max here probably will just cause the function to not converge and produce a less informative error).

I also added a robust::ratio to where we divide by Ts. It is possible for this to underflow which might be zero.

Copy link
Collaborator

@chadmeyer chadmeyer left a comment

Choose a reason for hiding this comment

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

All looks good to me now!

@jhp-lanl jhp-lanl merged commit 5229919 into main May 14, 2024
5 checks passed
@jhp-lanl jhp-lanl deleted the jhp/DavisRobust branch May 14, 2024 16:25
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