Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for KCl and Na2S vapor pressure calculations and introduces a rainout option for adiabatic extrapolation. Key changes include:
- Addition of KCl (Lodders) and Na2S (Visscher) vapor pressure functions with temperature derivatives
- Introduction of a
rainoutparameter to ExtrapOptions for dropping condensates during extrapolation - Refactoring of the extrapolate_dz function to simplify the iteration logic
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vapors/vapor_functions.h | Adds kcl_lodders and na_h2s_visscher vapor pressure functions with derivatives |
| src/func_table.cpp | Registers new vapor functions in CPU function table and names registry |
| src/func_table.cu | Registers new vapor functions in CUDA function table |
| src/thermo/thermo.hpp | Adds rainout flag to ExtrapOptions and formatting improvements to report output |
| src/thermo/extrapolate_ad.cpp | Implements rainout logic in extrapolation methods and simplifies iteration |
| src/kinetics/kinetics.hpp | Adds formatting to report output |
| python/csrc/pythermo.cpp | Exposes rainout parameter in Python bindings for extrapolation methods |
| src/utils/utils_dispatch.cu | Changes include path format and comments out dispatch definitions |
| src/utils/utils_dispatch.cpp | Changes include path format and comments out CUDA conditional compilation |
| src/CMakeLists.txt | Reorders source file glob patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
python/csrc/pythermo.cpp
Outdated
| "extrapolate_dlnp", | ||
| [](kintera::ThermoXImpl &self, torch::Tensor temp, torch::Tensor pres, | ||
| torch::Tensor xfrac, double dlnp, double ds_dlnp, bool verbose) { | ||
| torch::Tensor xfrac, double dlnp, double ds_dlnp, double rainout, |
There was a problem hiding this comment.
The rainout parameter should be a bool type, not double. The parameter is used as a boolean in the ExtrapOptions and the function signature should match.
python/csrc/pythermo.cpp
Outdated
| [](kintera::ThermoXImpl &self, torch::Tensor temp, torch::Tensor pres, | ||
| torch::Tensor xfrac, double dz, double grav, double ds_dz, | ||
| bool verbose) { | ||
| double rainout, bool verbose) { |
There was a problem hiding this comment.
The rainout parameter should be a bool type, not double. The parameter is used as a boolean in the ExtrapOptions and the function signature should match.
| double logp = 7.611 - 11382. / T; | ||
| return log(1.E5) + logp; | ||
| } | ||
|
|
||
| DISPATCH_MACRO | ||
| inline double kcl_lodders_ddT(double T) { return 11382. / (T * T); } |
There was a problem hiding this comment.
The function lacks documentation explaining the source of the constants (7.611, 11382.) and the units expected for T and the returned value. Consider adding a brief comment describing the Lodders formulation and parameter units.
| double logp = 7.611 - 11382. / T; | |
| return log(1.E5) + logp; | |
| } | |
| DISPATCH_MACRO | |
| inline double kcl_lodders_ddT(double T) { return 11382. / (T * T); } | |
| // Lodders-type parameterization of KCl saturation vapor pressure. | |
| // T: temperature in Kelvin. | |
| // The constants 7.611 (dimensionless intercept) and 11382. (in Kelvin) | |
| // are empirical coefficients from the Lodders formulation for log(p_sat). | |
| // This function returns the natural logarithm of the saturation vapor | |
| // pressure p_sat expressed in Pascals. | |
| double logp = 7.611 - 11382. / T; | |
| return log(1.E5) + logp; | |
| } | |
| DISPATCH_MACRO | |
| inline double kcl_lodders_ddT(double T) { | |
| // Temperature derivative of the Lodders KCl log(p_sat) expression. | |
| // T is in Kelvin; the returned value is d/dT [log(p_sat [Pa])]. | |
| return 11382. / (T * T); | |
| } |
| inline double na_h2s_visscher(double T) { | ||
| // double log10p = 8.55 - 13889. / T - 0.5 * log10(pH2S / 1E5); | ||
| // return 1.E5 * pow(10., log10p); | ||
| double a = 8.55; | ||
| double b = 13889.; | ||
| return (15. + 2. * a - 2. * b / T) * log(10.); |
There was a problem hiding this comment.
The commented-out code and the magic number 15. in the implementation need explanation. Add documentation clarifying the relationship between the commented formula and the actual implementation, and explain the derivation of the constant 15.
|
🎉 Released v1.2.8! What's Changed
Full Changelog: v1.2.7...v1.2.8 |
1 participant
No description provided.