Skip to content

Conversation

@tannerpolley
Copy link
Contributor

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:

  • Also adds bounds for phase equilibrium variables in general_property and smooth_VLE

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 91.11111% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.48%. Comparing base (8f96b37) to head (1569c0c).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...es/modular_properties/state_definitions/FpTPxpc.py 90.90% 10 Missing and 5 partials ⚠️
...erties/modular_properties/base/generic_property.py 66.66% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@dallan-keylogic dallan-keylogic left a 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.

Copy link
Contributor Author

@tannerpolley tannerpolley left a 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

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Dec 11, 2025
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants