Update idaes-pse requirement in preparation to 2.1 release #1034

lbianchi-lbl · 2023-05-18T20:24:45Z

TODO

Leave this PR open until the following PRs have been merged, and check that those changes are compatible with IDAES 2.1/Pyomo 6.6

Adding basic Electrolyzer unit model #1006
NaCl Property Pack with Temperature Dependence #1054
Adding costing method for rectifier capital cost and upscaled electricity flow #1057 (not likely to be affected)
Merging demo from NAWI Spring Meeting Pt2 - NF+Bypass example #1062 (once merged)

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.

andrewlee94 · 2023-05-25T20:30:56Z

The reason the tests are failing is because these specific tests are using the new Initilaizer objects in IDAES, but the property packages in use have not implemented the necessary fix_initialization_states methods.

This method needs to be implemented in the property packages, and needs to set the state blocks into a square form - generally fix the state variables and if necessary deactivate constraints that cannot be used when the states are fixed (e.g., sum of mole fractions). This is basically the same as the first step(s) of the older initialize methods. Note that this method does not need to return anything (unlike older implementations) as the Initializer is recording the initial state itself.

agarciadiego · 2023-06-14T23:20:26Z

@lbianchi-lbl I'm running this tests in IDAES 2.1 so some may fail if we are still using 2.0 in this branch. (haven't finished all the corrections)

setup.py

Co-authored-by: Ludovico Bianchi <[email protected]>

codecov · 2023-06-20T23:34:36Z

Codecov Report

Attention: Patch coverage is 85.36585% with 6 lines in your changes missing coverage. Please review.

Project coverage is 95.53%. Comparing base (e1fde51) to head (700b024).
Report is 276 commits behind head on main.

Files with missing lines	Patch %	Lines
watertap/property_models/NDMA_prop_pack.py	80.00%	1 Missing ⚠️
watertap/property_models/NaCl_T_dep_prop_pack.py	80.00%	1 Missing ⚠️
watertap/property_models/NaCl_prop_pack.py	80.00%	1 Missing ⚠️
watertap/property_models/cryst_prop_pack.py	80.00%	1 Missing ⚠️
watertap/property_models/seawater_prop_pack.py	80.00%	1 Missing ⚠️
watertap/property_models/water_prop_pack.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1034   +/-   ##
=======================================
  Coverage   95.53%   95.53%           
=======================================
  Files         318      318           
  Lines       30966    31002   +36     
=======================================
+ Hits        29584    29619   +35     
- Misses       1382     1383    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

agarciadiego · 2023-06-20T23:40:12Z

@lbianchi-lbl all the test pass in Windows and Linux. I don't know where the E ModuleNotFoundError: No module named 'nbconvert' is coming from

lbianchi-lbl · 2023-06-21T01:57:43Z

@lbianchi-lbl all the test pass in Windows and Linux. I don't know where the E ModuleNotFoundError: No module named 'nbconvert' is coming from

@agarciadiego that's a dependency that used to be included with IDAES, but has been removed as part of IDAES/idaes-pse#1133. Adding it back as a testing dependency should resolve those failures.

adam-a-a · 2023-06-21T02:42:59Z

setup.py

@@ -80,7 +80,7 @@
        # primary requirements for unit and property models
        # maintainers: switch to SPECIAL_DEPENDENCIES_FOR_RELEASE when cutting a release of watertap
        *SPECIAL_DEPENDENCIES_FOR_PRERELEASE,
-        "pyomo>=6.2,<6.6",  # (also needed for units in electrolyte database (edb))
+        "pyomo>=6.6.1",  # (also needed for units in electrolyte database (edb))


Suggested change

"pyomo>=6.6.1", # (also needed for units in electrolyte database (edb))

"pyomo<=6.6.1", # (also needed for units in electrolyte database (edb))

Any reason why this shouldn't be as suggested? Would tests fail using a version of pyomo below 6.6.1?

Yes, Pyomo 6.6.0 has known issues that affect us, namely Pyomo/pyomo#2844 and IDAES/idaes-pse#1191. Both have been resolved in Pyomo 6.6.1.

watertap/examples/flowsheets/case_studies/activated_sludge/ASM2D_flowsheet.py

adam-a-a · 2023-06-21T02:49:10Z

watertap/examples/flowsheets/case_studies/activated_sludge/tests/test_asm2d_flowsheet.py

        )
        assert value(model.fs.Treated.conc_mass_comp[0, "X_PHA"]) == pytest.approx(
-            2.1218e-9, abs=1e-4
+            8.981e-5, abs=1e-4


X_PHA changed by 4 orders of magnitude but am unsure why.

watertap/examples/flowsheets/case_studies/electroNP/electroNP_flowsheet.py

agarciadiego · 2023-06-21T13:27:12Z

@lbianchi-lbl @bknueven Do you have any objections to me skipping the mac tests for the tests that are failing?

lbianchi-lbl · 2023-06-21T13:50:50Z

@lbianchi-lbl @bknueven Do you have any objections to me skipping the mac tests for the tests that are failing?

No objections from me. Skip away!

…into idaes-2.1

bknueven

Obviously there's a few things to investigate here, but I would be in favor of going ahead with this as-is.

lbianchi-lbl · 2023-06-30T13:38:12Z

It seems that merging in #1062 resulted in some test failures on Windows.

pytest:
Jupyter notebook:

Any thoughts on what could be done to try to address these? @adam-a-a @avdudchenko (based on the commits on #1062).

lbianchi-lbl self-assigned this May 18, 2023

ksbeattie added the Priority:High High Priority Issue or PR label May 25, 2023

ksbeattie requested review from andrewlee94, adam-a-a and bknueven May 25, 2023 20:18

lbianchi-lbl added 2 commits May 30, 2023 13:15

Try updating idaes-pse requirement to watertap-org/idaes-pse:main

b56f0df

Try pyomo from current Pyomo/pyomo:main

f9dbbc9

lbianchi-lbl force-pushed the idaes-2.1 branch from b4800e0 to f9dbbc9 Compare May 30, 2023 18:20

Use 2.1.0rc0 for IDAES requirement

32dca48

lbianchi-lbl linked an issue Jun 1, 2023 that may be closed by this pull request

Resolve compatibility issues with IDAES 2.1.0/Pyomo 6.6.1 #1049

Closed

4 tasks

ksbeattie added the IDAES label Jun 8, 2023

Try the combination IDAES 2.0, Pyomo 6.6.1

ee84bc2

ksbeattie assigned agarciadiego Jun 8, 2023

adam-a-a and others added 4 commits June 14, 2023 10:38

dummy commit fixing a typo

951247e

dummy commit for demonstration

79da7f4

fixing unit and prop test

01c3045

running black

ecc171a

lbianchi-lbl commented Jun 14, 2023

View reviewed changes

setup.py Outdated Show resolved Hide resolved

agarciadiego and others added 10 commits June 14, 2023 19:25

Update setup.py

c062d94

Co-authored-by: Ludovico Bianchi <[email protected]>

property intialize error

be32768

amo 1690 add initialize

a42a8a4

scaling asm2d

230a283

scaling for linux

3eaa200

turning off tee

cf8424e

Merge branch 'main' into idaes-2.1

688efe6

electronP scaling

bd91cdd

asm2d scaling

9a2c8f3

changing units of aggragate

ca25f95

Add nbconvert as a testing requirement

b5d096e

adam-a-a reviewed Jun 21, 2023

View reviewed changes

watertap/examples/flowsheets/case_studies/activated_sludge/ASM2D_flowsheet.py Outdated Show resolved Hide resolved

adam-a-a reviewed Jun 21, 2023

View reviewed changes

watertap/examples/flowsheets/case_studies/electroNP/electroNP_flowsheet.py Show resolved Hide resolved

agarciadiego added 4 commits June 21, 2023 12:14

mac test skip

9e9490c

Merge branch 'idaes-2.1' of https://github.com/lbianchi-lbl/watertap …

3463dd1

…into idaes-2.1

running black

1f08db8

test skip

81c309e

lbianchi-lbl marked this pull request as ready for review June 22, 2023 20:47

Merge branch 'main' into idaes-2.1

bafad62

bknueven approved these changes Jun 23, 2023

View reviewed changes

lbianchi-lbl added 7 commits June 23, 2023 13:23

Merge branch 'main' into idaes-2.1

c28f436

Merge branch 'main' into idaes-2.1

2b925dc

Merge branch 'main' into idaes-2.1

5d1aa7b

Merge branch 'main' into idaes-2.1

7093184

Add fix_initialization_states() for new NaCl_T_dep prop model

35eaab4

Merge branch 'main' into idaes-2.1

5c957d2

Merge branch 'main' into idaes-2.1

7e3c9c8

lbianchi-lbl mentioned this pull request Jun 30, 2023

Fix OS-specific solver issues for NF flowsheet #1072

Closed

3 tasks

lbianchi-lbl added 3 commits June 30, 2023 14:09

Add ad-hoc temporary workaround for solver inconsistencies on WIndows

528ff66

Remove notebook with failures on Windows

bb67779

Update IDAES requirement to 2.1.0

700b024

lbianchi-lbl merged commit 665254c into watertap-org:main Jun 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update idaes-pse requirement in preparation to 2.1 release #1034

Update idaes-pse requirement in preparation to 2.1 release #1034

lbianchi-lbl commented May 18, 2023 •

edited

Loading

andrewlee94 commented May 25, 2023

agarciadiego commented Jun 14, 2023

codecov bot commented Jun 20, 2023 •

edited

Loading

agarciadiego commented Jun 20, 2023

lbianchi-lbl commented Jun 21, 2023

adam-a-a Jun 21, 2023

lbianchi-lbl Jun 21, 2023

adam-a-a Jun 21, 2023

agarciadiego commented Jun 21, 2023

lbianchi-lbl commented Jun 21, 2023

bknueven left a comment

lbianchi-lbl commented Jun 30, 2023 •

edited

Loading

	"pyomo>=6.6.1", # (also needed for units in electrolyte database (edb))
	"pyomo<=6.6.1", # (also needed for units in electrolyte database (edb))

Update idaes-pse requirement in preparation to 2.1 release #1034

Update idaes-pse requirement in preparation to 2.1 release #1034

Conversation

lbianchi-lbl commented May 18, 2023 • edited Loading

TODO

Legal Acknowledgement

andrewlee94 commented May 25, 2023

agarciadiego commented Jun 14, 2023

codecov bot commented Jun 20, 2023 • edited Loading

Codecov Report

agarciadiego commented Jun 20, 2023

lbianchi-lbl commented Jun 21, 2023

adam-a-a Jun 21, 2023

Choose a reason for hiding this comment

lbianchi-lbl Jun 21, 2023

Choose a reason for hiding this comment

adam-a-a Jun 21, 2023

Choose a reason for hiding this comment

agarciadiego commented Jun 21, 2023

lbianchi-lbl commented Jun 21, 2023

bknueven left a comment

Choose a reason for hiding this comment

lbianchi-lbl commented Jun 30, 2023 • edited Loading

lbianchi-lbl commented May 18, 2023 •

edited

Loading

codecov bot commented Jun 20, 2023 •

edited

Loading

lbianchi-lbl commented Jun 30, 2023 •

edited

Loading