-
Notifications
You must be signed in to change notification settings - Fork 21
pt derivatives from preferred #618
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
Changes from all commits
778a20b
6c4e65f
463ce74
1f2bcb9
8ad0b20
ed3dd80
3101b31
fc5367d
ea7f14f
08ff08c
b5ff9e3
4d3a781
fc2f701
1ec6dfe
e2a9e1b
bd59490
a56cf66
61da98d
3dabe6f
84cd524
cae1dc7
02bbf9b
24090dc
9f88d51
755fa84
00298ed
c10a468
2264aed
0f123dd
655a01b
129f626
a6ccf42
1c3832c
178d51d
194dbf7
153dbf5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -215,6 +215,14 @@ class SpinerEOSDependsRhoSieTransformable | |
| PORTABLE_INLINE_FUNCTION void | ||
| DensityEnergyFromPressureTemperature(const Real press, const Real temp, | ||
| Indexer_t &&lambda, Real &rho, Real &sie) const; | ||
| /* | ||
| // TODO(JMM): For now using FD. Fix this. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My second approach for EOSPAC could actually be the preferred method for spiner. Either that or you can fall back on the Menikoff and Plohr relations that rely on the derivatives that spiner already tabulates.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was putting off the spiner version because I was going to add additional tables possibly. |
||
| template <typename Lambda_t = Real *> | ||
| PORTABLE_INLINE_FUNCTION void | ||
| PTDerivativesFromPreferred(const Real rho, const Real sie, const Real P, const Real T, | ||
| Lambda_t &&lambda, Real &dedP_T, Real &drdP_T, Real &dedT_P, | ||
| Real &drdT_P) const; | ||
| */ | ||
| template <typename Indexer_t = Real *> | ||
| PORTABLE_INLINE_FUNCTION void | ||
| FillEos(Real &rho, Real &temp, Real &energy, Real &press, Real &cv, Real &bmod, | ||
|
|
||
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.
There are actually a few ways to skin this cat. Whichever you choose, I'd want a justification for why that's the method of choice. For example, I can think of two other ways that have significant advantages:
EOS_BTt_DTgets you the isothermal bulk modulus that can be converted intodrdP_TEOS_ALPHAt_DTgets you the thermal expansion coefficient, which can be converted todrdT_PEOS_CPt_DTandEOS_ALPHAt_DTwithEOS_BTt_DTandEOS_ALPHAt_DTwithThe first method is likely the fastest with only two lookups, but I would suspect that the second could be the most accurate, but will be at least twice as expensive (four lookups). Both are subject to the assumption of thermodynamic consistency of course. I would assume that the lookups in (2) are doing thermo identities under the hood, so it may actually not be more accurate if errors compound as EOSPAC does thermo identities and then you use those in a different relation to get different quantities. I'd lean towards the first method I present here, but I could be convinced that your method is superior (although it does involve a root find).
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.
Yeah... I think you're right that using the density of pressure/temperature table is overcomplicating things. I'll switch to path 1.
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'm happy to check the math when it's implemented 🙂
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.
It's implemented now. Please do! The more basic the math, the more likely I am to have completely messed it up.