idaes-pse
idaes-pse copied to clipboard
Complementarity-based VLE formulation
Fixes
Summary/Motivation:
This pull request seeks to add the new complementarity-based vapor-liquid equilibrium formulation with thermodynamic models supported by cubic equations of state. Currently, only the subcritical regime is activated.
Changes proposed in this PR:
- Addition of the new formulation and modifying the scripts
generic_property.py
,smooth_VLE.py
andceos.py
- Addition of critical volume and critical compressibility factor in the property packages
BT_PR.py
andASU_PR.py
- Addition of basic unit tests in
test_flash_PR.py
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution:
- I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
- 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.
A few general comments to get things started:
1. You need to run `black` on the code before committing, otherwise tests will not get run. I also noted that the branch is out of date with the current main. 2. Whilst I understand that it is easier to implement in parallel (rather than trying to update the existing code in one go), we should not be committing both versions side-by-side like this (especially not in the "core" parts of IDAES where we want to promise backwards compatibility). For now we should probably work on a branch whilst we refine everything. 3. This will also make reviewing the code a lot easier - as the files are currently all "new", it is hard to tell what you have changed here, and what is just copied from the original.
We deliberately decided not to modify existing code but take it in steps. What do you suggest we do?
@jghouse88 Working on a branch is probably best for now - the alternative would be to put it in models_extra
but I am not sure that would be any better. As I noted, a branch will also make it clearer what changes are being made to the files as we can easily see the diff.
However, we definitely should not be merging this into models
in the current state.
EDIT: Also, on a branch we can allow tests to fail in the interim period (if required), so we can work directly on the main files and use the test failure to track any issues that still need to be addressed.
@andrewlee94 We ideally want to deprecate the older smooth VLE formulation that uses the bubble/dew point. I would rather this go to the models folder than models extra and it languishing there without getting used. What would be necessary in addition to what @viiibhav has done here to get to models?
@viiibhav You and I will need to redo this PR to show what changes are happening from the previous version of the CEOS which means you will need to make changes directly to those files and not create a copy with _ncp
tag.
@viiibhav Also note you have some conflicts to fix (which you should probably do first before they get any worse). These should all be due to some changes I made in how the metadata for units are defined - before they were stored as a dict (derived_units["temperature"]
) but now they exist as @properties
(derived_units.TEMPERATURE
, which supports auto-completion). note that the old approach still works for backward compatibility, but the new approach is preferred.
Also be aware of PR #995, which does a similar thing for thermophsyical properties as well (although there is still a fair bit of work left for that PR to be ready).
@viiibhav any updates on this and/or resolving these conflicts?
@ksbeattie I will be pushing some more updates soon.
@viiibhav is this something that will be ready for the Feb release, meaning ready to merge in the next 2 weeks?
@ksbeattie probably not, but I will try and get to it.
@viiibhav is this now possible for the May release?
@ksbeattie yes it should be ready by then.
@viiibhav any news on this?
@ksbeattie Still working on it, was caught up with defense and graduation.
Moving this to the Aug release, in hopes it might get done by then.
No news as whether this is being worked on for the Aug release.
@viiibhav let us know if there are any updates on whether or when you'll be able to resume work on this.
@viiibhav closing this due to inactivity.