-
Notifications
You must be signed in to change notification settings - Fork 276
Helmholtz ammonia #1637
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
base: main
Are you sure you want to change the base?
Helmholtz ammonia #1637
Conversation
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.
Do you test the expression for thermal conductivity anywhere?
idaes/models/properties/general_helmholtz/components/parameters/co2.json
Show resolved
Hide resolved
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.
A couple comments on the documentation, plus I second @dallan-keylogic's request to ensure everything is tested.
docs/reference_guides/model_libraries/generic/property_models/helmholtz_parameters.rst
Show resolved
Hide resolved
docs/reference_guides/model_libraries/generic/property_models/helmholtz_parameters.rst
Outdated
Show resolved
Hide resolved
…helmholtz_parameters.rst Co-authored-by: Brandon Paul <[email protected]>
I did not as I could not find existing tests to follow for this within the package. This is also true for viscosity and surface tension. If there are some existing tests, please point me toward them and I will replicate them for Ammonia. Otherwise, if need be we can create some tests but it might be a month or so until we get around to it. |
It would be wonderful if you could create some tests. As we saw with CO2 using the surface tension value for water, an error can stay undetected for a long time unless we're explicitly testing against experimental values. But since you're contributing this voluntarily, it's not required to be merged. We could just merge this and open an issue to write these tests. @agarciadiego , what do you think? |
@StephenBurroughs , could you please merge main into your branch? That will fix the failing CoolProp test. |
…into HelmholtzAmmonia
Fixes
Adds Helmholtz formulation for ammonia based on SKehui Gao, Jiangtao Wu, Ian H. Bell, Allan H. Harvey, and Eric W. Lemmon (2023): " A Reference Equation of State with an Associating Term for Thermodynamic Properties of Ammonia" along with with thermal conductivity and viscosity rules
Also, fixes an existing issue where the CO2 was using a surface tension correlation value for water - Mulero, A., I. Cachadina, Parra, M., Recommended Correlations for the Surface Tension of Common Fluids, J. Phys. Chem. Ref. Data 41, 043105, (2012).
Summary/Motivation:
We needed to add Ammonia into the Ahuora Platform, seemed only right to also integrate it upstream!
Changes proposed in this PR:
Comparison between T/S and P/H plots generated within this property package and those from CoolProp:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: