Fix pcsaft flash #1913
Fix pcsaft flash #1913
Conversation
Pulling updates from CoolProp master
Pull changes from original repo
include/all_pcsaft_JSON.h
Outdated
| // File generated by the script dev/generate_headers.py on 2019-12-12 21:03:32.480501 | ||
| // File generated by the script dev/generate_headers.py on 2020-04-07 19:35:54.752805 | ||
|
|
There was a problem hiding this comment.
I should have noted this before the first PC-SAFT PR, but the autogenerated json files in the include folder should not be under source control
There was a problem hiding this comment.
I added this to the .gitignore file.
There was a problem hiding this comment.
Can you also remove the generated JSON files from source control? They aren't enormous, but every byte counts.
There was a problem hiding this comment.
Ah yes, I forgot to do that. Done.
| @@ -40,7 +40,9 @@ const parameter_info parameter_info_list[] = { | |||
| {iCvmass, "Cvmass", "O", "J/kg/K", "Mass specific constant volume specific heat", false}, | |||
| {iCp0molar, "Cp0molar", "O", "J/mol/K", "Ideal gas molar specific constant pressure specific heat",false}, | |||
| {iCp0mass, "Cp0mass", "O", "J/kg/K", "Ideal gas mass specific constant pressure specific heat",false}, | |||
| {iSmolar_residual, "Smolar_residual", "O", "J/mol/K", "Residual molar entropy (sr/R = tau*dar_dtau-ar)",false}, | |||
| {iHmolar_residual, "Hmolar_residual", "O", "J/mol/K", "Residual molar enthalpy",false}, | |||
| {iSmolar_residual, "Smolar_residual", "O", "J/mol/K", "Residual molar entropy (sr/R = s(T,rho) - s^0(T,rho))",false}, | |||
There was a problem hiding this comment.
Nice! This was long overdue anyhow.
| .value("iHmolar_residual", parameters::iHmolar_residual) | ||
| .value("iGmolar_residual", parameters::iGmolar_residual) | ||
| .value("iDmass", parameters::iDmass) |
There was a problem hiding this comment.
This is great! Can you perhaps also make the same changes to the cython files? The cython interface (for a variety of historical and non-historical reasons) is what is actually used with Python.
There was a problem hiding this comment.
I updated the AbstractState pyx and pxd files. Are there any other files I need to update?
include/all_pcsaft_JSON.h
Outdated
| // File generated by the script dev/generate_headers.py on 2019-12-12 21:03:32.480501 | ||
| // File generated by the script dev/generate_headers.py on 2020-04-07 19:35:54.752805 | ||
|
|
There was a problem hiding this comment.
Can you also remove the generated JSON files from source control? They aren't enormous, but every byte counts.
| cpdef double gibbsmolar_residual(self) except * | ||
| cpdef double hmolar_residual(self) except * | ||
| cpdef double smolar_residual(self) except * |
There was a problem hiding this comment.
I think they also need to get added to cAbstractState.pxd . This code duplication for C++ classes is one reason that I much prefer pybind11 for wrapping C++. Perhaps you can test compilation for Python?
There was a problem hiding this comment.
I added them to cAbstractState.pxd as well. I tested compilation and ran a small Python script to check that it worked. The PC-SAFT values matched those from the HEOS backend.
|
Thanks, merged. |
Requirements
Description of the Change
The flash and density solvers for the PC-SAFT backend were update to make them more reliable. Some minor edits to function names were also added to make it clear that PC-SAFT calculates residual H, S, and G.
Benefits
Improved consistency and higher likelihood that PC-SAFT backend will find the correct solution.
Possible Drawbacks
That code elsewhere might break
Verification Process
I added additional unit tests to confirm that the flash and density functions worked over a wider range of conditions, and all these unit tests passed. I also added a python script to make consistency plots for the PC-SAFT backend, and the consistency improved significantly after the changes. I also ran all the unit tests and almost all passed. Of 50497 assertions only 240 failed. Most of those that failed were because I do not have the Refprop or UNIFAC libraries, and the few others I checked also failed before the changes made here, so they appear to be unrelated to the changes here.
Applicable Issues
[ Enter any applicable Issues here. Use
Closes #????if this PR closes an open issue. ]