-
Notifications
You must be signed in to change notification settings - Fork 241
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 bugs in SmoothVLE2 initialization #1563
base: main
Are you sure you want to change the base?
Conversation
CubicComplementarityVLE.calculate_teq(m.props[1], ("Vap", "Liq")) | ||
assert not m.props[1].is_property_constructed("temperature_dew") | ||
assert not m.props[1].is_property_constructed("temperature_bubble") |
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.
It appears that the framework does not create these equilibrium properties because there is only one VLE component in the system, which makes sense. Are there tests for other scenarios, for example one or more non-condensable or non-volatile components in a system with two or more VLE components? In those cases, these properties should still be created.
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.
The SmoothVLE2
method doesn't need bubble and dew points to operate, unlike the original SmoothVLE
. The test coverage here is pretty poor, but I wanted to fix the outstanding issue quickly before moving onto other things.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1563 +/- ##
=======================================
Coverage 77.06% 77.06%
=======================================
Files 389 389
Lines 62680 62680
Branches 10276 10276
=======================================
+ Hits 48303 48306 +3
+ Misses 11939 11937 -2
+ Partials 2438 2437 -1 ☔ View full report in Codecov by Sentry. |
|
||
|
||
class TestWithNonCondensable: | ||
# TODO These tests could be fleshed out |
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.
What would this TODO entail?
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.
Mostly adding in tests for the values of gp
, gn
, _teq
, and other VLE variables both to make sure the model isn't changing and that the answer is reasonable. Right now, I'm testing that 1) the model initialization doesn't end up with an error and 2) bubble/dew points are not erroneously created.
Fixes
SmoothVLE2
when non-condensables are present.Summary/Motivation:
Addresses #1560 .
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: