-
Notifications
You must be signed in to change notification settings - Fork 283
Addition of the FpTPxpc state definition with required testing #1710
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?
Conversation
…nding the property
…se test. Uses a FeedFlash unit model rather than a state block to perform the test
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1710 +/- ##
==========================================
+ Coverage 77.45% 77.48% +0.02%
==========================================
Files 395 397 +2
Lines 64792 64992 +200
Branches 10902 10931 +29
==========================================
+ Hits 50186 50357 +171
- Misses 12095 12117 +22
- Partials 2511 2518 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dallan-keylogic
left a comment
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.
In general, this looks good. There are a few minor changes that are necessary. However, in addition to the old-style calculate_scaling_factors method, you should include a new style scaler object. The other state definitions now have scaler objects that you can use as an example.
idaes/models/properties/modular_properties/examples/tests/test_BTIdeal_FpTPxpc.py
Outdated
Show resolved
Hide resolved
idaes/models/properties/modular_properties/examples/tests/test_BTIdeal_FpTPxpc.py
Outdated
Show resolved
Hide resolved
idaes/models/properties/modular_properties/examples/BT_ideal_FpTPxpc.py
Outdated
Show resolved
Hide resolved
idaes/models/properties/modular_properties/state_definitions/tests/test_FpTPxpc.py
Outdated
Show resolved
Hide resolved
…ant config properties dictionary and file
tannerpolley
left a comment
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.
Resolved comments made by Doug
…nstraint' based on `has_phase_equilibrium` boolean. Also additional conditional in blocking the activiation of the `equilibrium_constraint` if `mole_frac_phase_comp` is a state variable
Fixes
Adds the state definition FpTPxpc with required testing
Summary/Motivation:
There were cases where the FpcTP state definition was failing with Modular Properties
Changes proposed in this PR:
general_propertyandsmooth_VLELegal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: