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

PowerMG EOS implemented and documented. #336

Merged
merged 9 commits into from
Feb 1, 2024
Merged

PowerMG EOS implemented and documented. #336

merged 9 commits into from
Feb 1, 2024

Conversation

aematts
Copy link
Collaborator

@aematts aematts commented Jan 22, 2024

PR Summary

The Power Mie-Gruneisen EOS created at SNL by Robinson, adequate for high pressure modeling, is implemented and documented in relation to the linear Us-up Mie-Gruneisen implemented previously. This EOS is a full EOS with correct temperature. It is the least restrictive EOS of Mie-Gruneisen form and similar in spirit to the Extended Vinet EOS.

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.
  • 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

@aematts aematts self-assigned this Jan 22, 2024
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.

Thanks for this, @aematts ! Assuming tests pass, this looks good to me. I have a few minor comments, but they are suggestions, and I consider them non-blocking. Feel free to push back on them.

singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Show resolved Hide resolved
@aematts
Copy link
Collaborator Author

aematts commented Jan 23, 2024

OK. I have fixed a few small things (initiating _M, and adding const in front of Real eta = ....) and of course forgot to rerun the formatting.

@aematts
Copy link
Collaborator Author

aematts commented Jan 23, 2024

Last comments for today:

  1. I could go through and make this more efficient by using one routine for everything, as I did in Vinet (Vinet_F_DT_func) this would avoid calculating the same thing several times in FillEos (as Jonah pointed out). But this is not how Robinson has written it in the report, so it would take some work. But I actually think it is a god idea for eos implementation since it exposes everything that is not consistent. Maybe we can put it up as a future wish.
  2. The temperature convergence: in _SN2Mp2 I have set maxind to 200 so that the while loop would never be able to go on forever. I am pretty sure that this is sufficient for everything, but I put in a PORTBLE_WARN in case it was not. Is this an OK solution or should we discuss it more?
  3. Tension: Always a tricky question and the solution Robinson has in his report is as good as anything, in my view. But it does give negative bulk modulus and such things. Maybe we should discuss this more, but then maybe in general terms to compare different solutions different people have to this.
    That's it for today.

@Yurlungur
Copy link
Collaborator

Yurlungur commented Jan 23, 2024

  1. I could go through and make this more efficient by using one routine for everything, as I did in Vinet (Vinet_F_DT_func) this would avoid calculating the same thing several times in FillEos (as Jonah pointed out). But this is not how Robinson has written it in the report, so it would take some work. But I actually think it is a god idea for eos implementation since it exposes everything that is not consistent. Maybe we can put it up as a future wish.

Wishlist item is good. Don't worry about it for now. :)

  1. The temperature convergence: in _SN2Mp2 I have set maxind to 200 so that the while loop would never be able to go on forever. I am pretty sure that this is sufficient for everything, but I put in a PORTBLE_WARN in case it was not. Is this an OK solution or should we discuss it more?

This is an OK solution for now. We can revisit if we actually hit the limit. You could move the limit to a global const (e.g., static constexpr int MAXITER=200 in the class definition) if you like, that could serve as a useful reminder to look at it if we hit problems.

  1. Tension: Always a tricky question and the solution Robinson has in his report is as good as anything, in my view. But it does give negative bulk modulus and such things. Maybe we should discuss this more, but then maybe in general terms to compare different solutions different people have to this.

This I'm not sure about and I'm curious what @jhp-lanl thinks. I worry a bit about the EOS returning things like negative bulk modulus, as a hydro code that receives something like that might go completely bonkers. I don't think this necessarily has to be solved now---it could be resolved, for example, with friendly testing from a host code, if we have people interested in using the EOS.

@aematts
Copy link
Collaborator Author

aematts commented Jan 23, 2024

Tension: Note that the Pmin is only used at very large tension. The expansion first go through a part that follows the isentrope.
image

@aematts
Copy link
Collaborator Author

aematts commented Jan 23, 2024

In fact if you set a large negative value for Pmin (as the default) it never really kicks in but follows the isentrope all the way to infinite volume ;).

@Yurlungur
Copy link
Collaborator

That figure looks totally reasonable. And it's not like our tabulated EOS's don't also have crazy stuff in tension.

Copy link
Collaborator

@jhp-lanl jhp-lanl left a comment

Choose a reason for hiding this comment

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

Some minor comments and suggested changes.

It's a shame the HTML can't render a PDF. I'm fine with the hyperlinks, so my suggestions are mainly to try to make it more obvious what the hyperlinks are doing and to separate them from the text.

doc/sphinx/src/models.rst Outdated Show resolved Hide resolved
doc/sphinx/src/models.rst Outdated Show resolved Hide resolved
doc/sphinx/src/models.rst Outdated Show resolved Hide resolved
doc/sphinx/src/models.rst Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Outdated Show resolved Hide resolved
singularity-eos/eos/eos_powermg.hpp Outdated Show resolved Hide resolved
@aematts
Copy link
Collaborator Author

aematts commented Jan 31, 2024

This is ready to merge now.

@jhp-lanl
Copy link
Collaborator

I'm running a pipeline on re-git to make sure things work there

@jhp-lanl
Copy link
Collaborator

jhp-lanl commented Feb 1, 2024

I'm running a pipeline on re-git to make sure things work there

Pipeline passes on re-git... merging

@jhp-lanl jhp-lanl merged commit 368ae81 into main Feb 1, 2024
4 checks passed
@jhp-lanl jhp-lanl deleted the aemattssing branch February 1, 2024 03:02
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