-
Notifications
You must be signed in to change notification settings - Fork 59
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
Update IDAES requirement to 2.2.0.dev0.watertap.23.08.03 and other stability improvements #930
Merged
Merged
Changes from 62 commits
Commits
Show all changes
68 commits
Select commit
Hold shift + click to select a range
57eea20
Revert "switch to nl_v1 (for now)"
bknueven c02dec2
doing some debugging
bknueven e321ead
Merge branch 'main' into issue929
bknueven 6a209fd
Modify and rescaled following vars and constraints for chemistry test…
9c41335
Update tests for chem_scaling_utils modifications
09ac4c8
Enthalpy unit corrected for EDB data
c31cd09
chem_scaling_utils updated for corrected enthalpy
d16fc70
Temperature bound modified to avoid breaking out the solvent critical…
21b5c69
Correct enthalpy units and update assertion in all tests, all pass
15b4f09
Revert "switch to nl_v1 (for now)"
bknueven 4ca862c
doing some debugging
bknueven 4ed5c2e
Modify and rescaled following vars and constraints for chemistry test…
216f7f4
Update tests for chem_scaling_utils modifications
c6011bc
Enthalpy unit corrected for EDB data
ae9a156
chem_scaling_utils updated for corrected enthalpy
f358836
Temperature bound modified to avoid breaking out the solvent critical…
a54427c
Correct enthalpy units and update assertion in all tests, all pass
4be6732
Chemistry tests updated after adding mutable parameters to reformulat…
f1035c4
Correct TNK to use components updated in anaerobic environment, break…
7cd9b05
Update documentation for asm1_adm1 translator
b207563
Typo/equations corrected for asm1_adm1 translator documentation
bc2e2a7
Merge remote-tracking branch 'upstream/main' into issue929
bknueven 0aab640
Merge remote-tracking branch 'upstream/main' into lxhowl_issue929
bknueven 1f64faa
Merge branch 'lxhowl_issue929' into issue929
bknueven 564104e
black
bknueven c84efd3
remove errant merge conflict
bknueven d83dc38
pylint
bknueven 4907892
Merge branch 'main' into issue929
bknueven 72d4f16
using idaes-pse with Xinhong's fixes
bknueven 7b74ae0
pylint
90dd26c
Fix NaOCl flowsheet
494faa9
Remode model.clone in gac test
c5af80b
Update scaling in ion exchange test
debc6cd
Merge pull request #3 from lxhowl/issue929
bknueven 37718f5
Update IDAES requirement to 2.2.0.dev0.watertap.23.08.03 tag
lbianchi-lbl 9160ef1
Merge remote-tracking branch 'lbianchi-lbl/update-idaes-2.2.0.dev0' i…
bknueven bd39f51
Test NaOCl flowsheet on linux
a5fe549
Test 2 NaOCl flowsheet on linux
868abb4
NF w/ bypass flosheet test
b8d1d89
Add scaling option
1a74626
Merge pull request #4 from lxhowl/issue929
bknueven a5be500
run black
25617a4
Merge pull request #5 from lxhowl/issue929
bknueven 06db0a9
Fix pylint
83f4a1e
Fix docs test
259ea4a
Remove skip for nf flowsheet notebook
55c846e
Merge pull request #6 from lxhowl/issue929
bknueven 427f56f
Merge branch 'main' into issue929
bknueven e59323c
Skip macOS failed tests
cfd3805
Import pytest to skip ui tests for macOS
0305d01
test ui tests
28c7d88
run black
1b8b04b
Merge pull request #7 from lxhowl/issue929
bknueven ddde7f7
cleanup nf_with_bypass_ui
bknueven f86506d
skip nf_with_bypass_ui test on macOS
bknueven 3d90146
Merge remote-tracking branch 'upstream/main' into issue929
bknueven 464e57b
Merge branch 'main' into issue929
bknueven 31bd11b
Merge branch 'main' into issue929
bknueven b84c490
pydantic<2
bknueven 9d903f8
Merge branch 'issue929' of github.com:bknueven/proteuslib into issue929
bknueven edf0d35
Merge branch 'main' into issue929
adam-a-a 42a4803
Update watertap/examples/chemistry/tests/test_pure_water_pH.py
adam-a-a 948d410
Update watertap/examples/chemistry/tests/test_solids.py
adam-a-a 52efaf5
run black
adam-a-a fd8cca4
remove debugging statement
bknueven 64a4e03
Merge branch 'main' into issue929
bknueven 1a130cb
cleanup pylint failure from #1091
bknueven 0adcc9c
Merge branch 'main' into issue929
adam-a-a File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,12 @@ A portion of the displayed output is shown below. | |
|
||
.. testoutput:: | ||
|
||
WARNING: model contains export suffix 'fs.state_block[0].scaling_factor' that | ||
contains 4 component keys that are not exported as part of the NL file. | ||
Skipping. | ||
WARNING: model contains export suffix 'fs.state_block[0].scaling_factor' that | ||
contains 4 component keys that are not exported as part of the NL file. | ||
Skipping. | ||
Comment on lines
+73
to
+78
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like something we wouldn't want showing up in the how-to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Block fs.state_block[0] | ||
|
||
Variables: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ | |
# update with a tag from the nawi-hub/idaes-pse | ||
# when a version of IDAES newer than the latest stable release from PyPI | ||
# will become needed for the watertap development | ||
"idaes-pse==2.1.0", | ||
"idaes-pse @ git+https://github.com/watertap-org/[email protected]", | ||
] | ||
|
||
# Arguments marked as "Required" below must be included for upload to PyPI. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
"temperature": [ | ||
273.15, | ||
300, | ||
650 | ||
647 | ||
], | ||
"pressure": [ | ||
50000, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I suppose the alternative GUI flowsheet code was also causing checks to fail, hence it's addition here. DOes that mean the UI code for these NF flowsheets still has issues to address?
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.
IIRC this is due to a limitation of the
watertap.ui.fsapi
layer for models that require the "good" (ma27
-enabled) Ipopt to solve. If the model can be initialized without failures, specifyingrequires_idaes_solver=True
kwarg is enough to skip the corresponding test without failures, but this doesn't work when the solve fails during initialization. Therefore, we have to exclude the entire test module using the-k not ...
pytest flag.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.
Note -- this only applies to the macOS tests