-
-
Notifications
You must be signed in to change notification settings - Fork 346
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 Redlich-Kwong partial molar properties #1218
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1218 +/- ##
=======================================
Coverage 65.43% 65.44%
=======================================
Files 320 320
Lines 46249 46250 +1
Branches 19657 19659 +2
=======================================
+ Hits 30265 30267 +2
+ Misses 13454 13453 -1
Partials 2530 2530
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
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.
Thanks for tackling this one, @speth. It looks good to me.
Is the idea that hopefully some day we will remove the check_cp
flag from the checkThermo
definition? I think it's the right call, right now, but does seem odd that just that specific property is excepted (and I certainly wouldn't want it viewed as a precedent for anyone who just doesn't feel like implementing a full thermo feature set, but I think that's easy enough for us to head off).
getPartialMolarEnthalpies(ubar); | ||
double p = pressure(); | ||
for (size_t k = 0; k < nSpecies(); k++) { | ||
ubar[k] -= p * m_partialMolarVolumes[k]; |
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.
Nice! I was fearing some long derivation on which I'd have to go and run due diligence to check 😂
2642061
to
c72f095
Compare
Ah, that's an interesting point -- as I had it, it would have probably gone unnoticed to remove the I'm not necessarily worried about thermo models not providing a "complete" interface. I'd be more interested in defining what the minimal interface that a model needs to provide is, say to be compatible with the reactor network or 1D flame models. As it is, I don't know when you actually need to know the partial molar specific heats -- they appear in the 1D flame for the ideal gas case, but end up being replaced by the partial molar enthalpies in the non-ideal formulation (according to #1079). |
Ah, good point - the partial molar enthalpies being the generally correct approach, I would be slightly curious to see if there is a speed difference, or if we could just replace the partial molar specific heats in the ideal gas reactor with partial molar enthalpies, and be no worse for it. IIRC, one of J. Shepherd's students originally wrote asking for access to these partial molar entities, for use with non-ideal phases in |
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.
@decaluwe ... is this ready to be merged? If not, just dismiss my approval.
Changes proposed in this pull request
RedlichKwongMFTP::getPartialMolarCp
RedlichKwongMFTP::getPartialMolarIntEnergies
If applicable, fill in the issue number this pull request is fixing
Closes #998
Checklist
scons build
&scons test
) and unit tests address code coverage