-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix bug in Gruneisen DensityEnergyFromPressureTemperature()
#403
Conversation
@chadmeyer and @jonahm-LANL can you review? |
Found another bug... working on it. Will make draft for now |
@chadmeyer and @Yurlungur I had to make a few more changes. It looks like the primary issue is that the Gruneisen EOS pressure is not monotonic along an isotherm. I didn't realize that subtraction of the reference energy curve will eventually overwhelms the pressure reference curve, causing a maximum in the pressure. I introduced a function to calculate the location of this maximum and essentially bound the EOS to the region up to this maximum (in P-T space). We could think about imposing this maximum on the other lookups too, but it would add an expensive root find. The P-T lookup is most likely at initialization so it's okay to be a bit more expensive there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I do think we should try to limit root finding generically in equations of state. However, it probably suggests each EOS should have additional introspection.
Before we merge I would like Chad's eyes on this. @chadmeyer
I'm converting back to draft. I'm encountering a situation where |
More specifically, the reference pressure in the beginning of the algorithm is of order -1e-13. Something is very weird here maybe in how the EOS parameters are being set up. |
I figured out the issue... it was a unit conversion issue with the input specification. I think this MR is fine. |
@chadmeyer please take a look |
I will observe here that RE of PT is generally going to be a root find for most EOSs, and isn't meant to be called much outside of certain special cases (i.e. not in an inner loop outside of initialization/remapping of an EOS). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this seems like a reasonable change, given the issue seen. Is there a more thermodynamically consistent form of the Gruneisen we could implement at some point?
@chadmeyer , we actually already have one that @aematts provided in eos_mgusup.hpp (documentation). The only downside of that EOS is that it is limited to Ann has also provided a power law Mie-Gruneisen that offers more flexibility in the shape of the Hugoniot reference curve, but could not use the Steinberg parameters directly. I think this version also has restrictions on the functional form of the Gruneisen parameter, possibly limiting accuracy off of the principal Hugoniot (re-shocks or rarefactions). More generally though, I think the only way to make this EOS thermodynamically consistent is to numerically integrate a temperature reference curve (i.e. the temperature along the Hugoniot reference curve). You could imagine doing this a priori and then either fitting it to a function or tabulating it (and maybe storing it in a spiner table). You could also do this in-line, but that would probably be expensive. |
It's worth pointing out that this Gruneisen is specifically in the code because it's not thermodynamically consistent but it is consistent with several host code implementations. |
@jhp-lanl , @chadmeyer , @Yurlungur : It is true that all three of the EOS's I have implemented uses I also want to mention the Extended Vinet EOS that is based on an isotherm (any temperature) and might be better for off-Hugoniot cases. It is a full EOS and not the cold curve form that is commonly named Vinet at LANL. |
I completely agree. Hugoniot reference curves are in general less flexible, and if calibrations exist, an isothermal reference curve EOS is preferable. |
One last edge case I found: if the reference temperature is absurd (i.e. a unit conversion issue), it's possible for the maximum pressure to occur at a density less than the reference density. In this case, all compression states are unstable, which is bogus. I added an error to help with this case. Will merge when tests pass. |
PR Summary
@Gerbeset wasn't able to use the
DensityEnergyFromPressureTemperature()
function with ambient conditions and properties for aluminum. This pointed to a bug in the implementation.As far as I can tell, the bug in question was in the bounds for the root find.The big issue I think was that the upper bound was outside the applicable maximum density of the EOS. Now that we have a maximum density implemented, this can bound the root find appropriately.EDIT - It turns out that the issue was thermodynamic consistency. For a given temperature, there is a maximum in the pressure since the cold curve energy doesn't increase with increasing density. As a result, at temperatures/energies below the reference curve, there will be a regime where the pressure decreases as a function of density. This is not a phase change, but rather a result of thermodynamic consistency breaking. The root find assumed a monotonic$P(rho, T)$ curve for a given temperature, so it was failing to converge.
See above for a plot of the pressure for various isotherms. This should be monotonic.
This MR introduces a new function that bounds the density to a range where the EOS is thermodynamically stable when performing the P-T lookup.
An open question is whether to apply this domain limiting procedure to all lookups or not.
Less important changes
I also switched from a reference capture lambda to our
PORABLE_LAMBDA
.To make the implementation easier to follow (and more general), I decided to just use density-temperature lookups everywhere. As a result, I find the density that satisfies the pressure-temperature point first and then use that density in the energy lookup. This isn't strictly necessary since the EOS assumes that energy is constant along an isotherm in density space (violating thermodynamic consistency), but I figured I'd do it this way anyway to be more correct. I think there should be negligible performance difference if any. Since this is generally an initialization function, I doubt that matters.
I also added a test to make sure that the call is returning correct values.
PR Checklist
make format
command after configuring withcmake
.