-
-
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 consistency of IAPWS water EoS #1394
Conversation
Partially resolves Cantera#1315
I have no idea what's causing the Windows 2019 failures, that wouldn't also be the same on Windows 2022. Setuptools did release a new version today that updates distutils, but that doesn't seem to be fixed by capping the version of I was trying to test over in #1392 what happens when SCons is < 4.4.0 and you have the Also, the example failure is due to a new warning from Matplotlib 3.6.0, and us setting the environment variable to treat warnings as errors. I fixed that in #991 by installing Matplotlib <3.6.0, but it may be worth restricting the environment variable to only treat Cantera-generated warnings as errors. I'm also very curious what you're working on that this was a priority to fix 😄 Unless it's just a general "well, I see how to fix this, so might as well" 😂 |
Codecov Report
@@ Coverage Diff @@
## main #1394 +/- ##
==========================================
- Coverage 71.08% 71.03% -0.06%
==========================================
Files 363 363
Lines 51839 51855 +16
Branches 17354 17362 +8
==========================================
- Hits 36849 36834 -15
- Misses 12645 12676 +31
Partials 2345 2345
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This isn't impacting anything specific I'm working on. Mostly just trying to make a little progress on the pile of bugs that were uncovered by #1299. I thought this was low-hanging fruit after figuring out the first one (scaling by R instead of RT), and then realized there were still some inexplicable discrepancies which were trickier to hunt down. |
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.
@speth … thanks for taking on some of the bugs. The code changes make sense to me (caveat: I have not reviewed underlying equations and am relying on the thermodynamics-inspired tests that uncovered the bug in the first place); I only have minor comments.
The windows 2022 runner works a little different, as it builds in a conda environment. |
One other alternative would be to create a pytest runner that handles the examples, so we get access to the warnings filters. PS: neither of the issues are related to the bug fix in this PR … |
The nondimensional EOS should always be dimensionalized to a mass basis using the value of R given in the original source. Values on a molar basis can then be computed consistently using the best available value for the molecular weight of water. Partially resolves Cantera#1315
I'm going to pass on trying to fix the pre-existing CI failures as part of this PR. I briefly tried adjusting the |
4b02935
to
64225fa
Compare
Remove code introduced in Cantera#1394
Remove code introduced in Cantera#1394
Remove code introduced in #1394
Remove code introduced in Cantera#1394
Remove code introduced in Cantera#1394
Changes proposed in this pull request
WaterSSTP
(was off by a factor ofA number of regression test values have changed, generally by factors on the order of$18.015268/18.015\approx 1.0000148$ which is the ratio of the molecular weight for water corresponding to the IAPWS95 value for $R$ and Cantera's molecular weight for water.
If applicable, fill in the issue number this pull request is fixing
Closes #1315
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build
&scons test
) and unit tests address code coverage