Skip to content
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 68 commits into from
Aug 19, 2023
Merged
Show file tree
Hide file tree
Changes from 63 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 Feb 17, 2023
c02dec2
doing some debugging
bknueven Mar 30, 2023
e321ead
Merge branch 'main' into issue929
bknueven May 17, 2023
6a209fd
Modify and rescaled following vars and constraints for chemistry test…
May 26, 2023
9c41335
Update tests for chem_scaling_utils modifications
Jun 5, 2023
09ac4c8
Enthalpy unit corrected for EDB data
Jun 7, 2023
c31cd09
chem_scaling_utils updated for corrected enthalpy
Jun 7, 2023
d16fc70
Temperature bound modified to avoid breaking out the solvent critical…
Jun 7, 2023
21b5c69
Correct enthalpy units and update assertion in all tests, all pass
Jun 7, 2023
15b4f09
Revert "switch to nl_v1 (for now)"
bknueven Feb 17, 2023
4ca862c
doing some debugging
bknueven Mar 30, 2023
4ed5c2e
Modify and rescaled following vars and constraints for chemistry test…
May 26, 2023
216f7f4
Update tests for chem_scaling_utils modifications
Jun 5, 2023
c6011bc
Enthalpy unit corrected for EDB data
Jun 7, 2023
ae9a156
chem_scaling_utils updated for corrected enthalpy
Jun 7, 2023
f358836
Temperature bound modified to avoid breaking out the solvent critical…
Jun 7, 2023
a54427c
Correct enthalpy units and update assertion in all tests, all pass
Jun 7, 2023
4be6732
Chemistry tests updated after adding mutable parameters to reformulat…
Jun 15, 2023
f1035c4
Correct TNK to use components updated in anaerobic environment, break…
Jun 26, 2023
7cd9b05
Update documentation for asm1_adm1 translator
Jun 26, 2023
b207563
Typo/equations corrected for asm1_adm1 translator documentation
Jun 27, 2023
bc2e2a7
Merge remote-tracking branch 'upstream/main' into issue929
bknueven Jun 28, 2023
0aab640
Merge remote-tracking branch 'upstream/main' into lxhowl_issue929
bknueven Jun 28, 2023
1f64faa
Merge branch 'lxhowl_issue929' into issue929
bknueven Jun 28, 2023
564104e
black
bknueven Jun 28, 2023
c84efd3
remove errant merge conflict
bknueven Jun 28, 2023
d83dc38
pylint
bknueven Jun 28, 2023
4907892
Merge branch 'main' into issue929
bknueven Aug 1, 2023
72d4f16
using idaes-pse with Xinhong's fixes
bknueven Aug 2, 2023
7b74ae0
pylint
Aug 2, 2023
90dd26c
Fix NaOCl flowsheet
Aug 2, 2023
494faa9
Remode model.clone in gac test
Aug 2, 2023
c5af80b
Update scaling in ion exchange test
Aug 2, 2023
debc6cd
Merge pull request #3 from lxhowl/issue929
bknueven Aug 2, 2023
37718f5
Update IDAES requirement to 2.2.0.dev0.watertap.23.08.03 tag
lbianchi-lbl Aug 3, 2023
9160ef1
Merge remote-tracking branch 'lbianchi-lbl/update-idaes-2.2.0.dev0' i…
bknueven Aug 3, 2023
bd39f51
Test NaOCl flowsheet on linux
Aug 3, 2023
a5fe549
Test 2 NaOCl flowsheet on linux
Aug 3, 2023
868abb4
NF w/ bypass flosheet test
Aug 3, 2023
b8d1d89
Add scaling option
Aug 3, 2023
1a74626
Merge pull request #4 from lxhowl/issue929
bknueven Aug 3, 2023
a5be500
run black
Aug 3, 2023
25617a4
Merge pull request #5 from lxhowl/issue929
bknueven Aug 3, 2023
06db0a9
Fix pylint
Aug 3, 2023
83f4a1e
Fix docs test
Aug 3, 2023
259ea4a
Remove skip for nf flowsheet notebook
Aug 3, 2023
55c846e
Merge pull request #6 from lxhowl/issue929
bknueven Aug 3, 2023
427f56f
Merge branch 'main' into issue929
bknueven Aug 3, 2023
e59323c
Skip macOS failed tests
Aug 4, 2023
cfd3805
Import pytest to skip ui tests for macOS
Aug 4, 2023
0305d01
test ui tests
Aug 5, 2023
28c7d88
run black
Aug 5, 2023
1b8b04b
Merge pull request #7 from lxhowl/issue929
bknueven Aug 9, 2023
ddde7f7
cleanup nf_with_bypass_ui
bknueven Aug 9, 2023
f86506d
skip nf_with_bypass_ui test on macOS
bknueven Aug 9, 2023
3d90146
Merge remote-tracking branch 'upstream/main' into issue929
bknueven Aug 9, 2023
464e57b
Merge branch 'main' into issue929
bknueven Aug 9, 2023
31bd11b
Merge branch 'main' into issue929
bknueven Aug 17, 2023
b84c490
pydantic<2
bknueven Aug 17, 2023
9d903f8
Merge branch 'issue929' of github.com:bknueven/proteuslib into issue929
bknueven Aug 17, 2023
edf0d35
Merge branch 'main' into issue929
adam-a-a Aug 18, 2023
42a4803
Update watertap/examples/chemistry/tests/test_pure_water_pH.py
adam-a-a Aug 18, 2023
948d410
Update watertap/examples/chemistry/tests/test_solids.py
adam-a-a Aug 18, 2023
52efaf5
run black
adam-a-a Aug 18, 2023
fd8cca4
remove debugging statement
bknueven Aug 18, 2023
64a4e03
Merge branch 'main' into issue929
bknueven Aug 18, 2023
1a130cb
cleanup pylint failure from #1091
bknueven Aug 18, 2023
0adcc9c
Merge branch 'main' into issue929
adam-a-a Aug 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,6 @@ jobs:
echo '::group::Output of "idaes get-extensions" command'
idaes get-extensions --verbose
echo '::endgroup::'
- name: Exclude notebooks that cause errors on Windows
if: startswith(matrix.os, 'win')
run: |
rm tutorials/nawi_spring_meeting2023.ipynb
- name: Run pytest with nbmake
run:
pytest --nbmake **/*.ipynb
Expand Down Expand Up @@ -304,4 +300,4 @@ jobs:
pyomo build-extensions || python -c "from pyomo.contrib.pynumero.asl import AmplInterface; exit(0) if AmplInterface.available() else exit(1)"
- name: Run pytest
run: |
pytest --pyargs watertap -k 'not nf_dspmde.nf_ui'
pytest --pyargs watertap -k 'not (nf_dspmde.nf_ui or nf_dspmde.nf_with_bypass_ui)'
Copy link
Contributor

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?

Copy link
Contributor

@lbianchi-lbl lbianchi-lbl Aug 18, 2023

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, specifying requires_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.

Copy link
Contributor Author

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

6 changes: 6 additions & 0 deletions docs/how_to_guides/how_to_use_a_property_model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It's just in the test -- so the reader won't see it.
  2. I talked with @andrewlee94 about this a few weeks ago and there is not a workaround nor is one planned (IDAES and Pyomo are implementing a new model scaling framework)

Block fs.state_block[0]

Variables:
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 0 additions & 6 deletions watertap/core/plugins/solvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#################################################################################

import pyomo.environ as pyo
from pyomo.opt import WriterFactory
from pyomo.core.base.block import _BlockData
from pyomo.core.kernel.block import IBlock
from pyomo.solvers.plugins.solvers.IPOPT import IPOPT
Expand All @@ -25,7 +24,6 @@
from idaes.logger import getLogger

_log = getLogger("watertap.core")
_default_nl_writer = WriterFactory.get_class("nl")


@pyo.SolverFactory.register(
Expand Down Expand Up @@ -56,9 +54,6 @@ def _presolve(self, *args, **kwds):
if "constr_viol_tol" not in self.options:
self.options["constr_viol_tol"] = 1e-08

# temporarily switch to nl_v1 writer
WriterFactory.register("nl")(WriterFactory.get_class("nl_v1"))

if not self._is_user_scaling():
self._cleanup_needed = False
return super()._presolve(*args, **kwds)
Expand Down Expand Up @@ -152,7 +147,6 @@ def _presolve(self, *args, **kwds):
raise

def _cleanup(self):
WriterFactory.register("nl")(_default_nl_writer)
if self._cleanup_needed:
self._reset_scaling_factors()
self._reset_bounds()
Expand Down
8 changes: 0 additions & 8 deletions watertap/core/plugins/tests/test_solvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import pyomo.environ as pyo
import idaes.core.util.scaling as iscale

from pyomo.opt import WriterFactory
from pyomo.solvers.plugins.solvers.IPOPT import IPOPT
from pyomo.common.errors import ApplicationError
from idaes.core.util.scaling import (
Expand All @@ -24,8 +23,6 @@
from idaes.core.solvers import get_solver
from watertap.core.plugins.solvers import IpoptWaterTAP

_default_nl_writer = WriterFactory.get_class("nl")


class TestIpoptWaterTAP:
@pytest.fixture(scope="class")
Expand Down Expand Up @@ -63,11 +60,6 @@ def _test_bounds(self, m):
def s(self):
return pyo.SolverFactory("ipopt-watertap")

@pytest.mark.unit
def test_nl_writer_held_harmless(self, m, s):
s.solve(m, tee=True)
assert _default_nl_writer == WriterFactory.get_class("nl")

@pytest.mark.unit
def test_pyomo_registration(self, s):
assert s.__class__ is IpoptWaterTAP
Expand Down
2 changes: 1 addition & 1 deletion watertap/edb/data/base.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"temperature": [
273.15,
300,
650
647
],
"pressure": [
50000,
Expand Down
28 changes: 14 additions & 14 deletions watertap/edb/data/component.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -542.83,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -208,7 +208,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -677.1,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -538,7 +538,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -1292.1,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -764,7 +764,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -240.1,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1132,7 +1132,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -1277.4,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1178,7 +1178,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -909.2,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1853,7 +1853,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -48.5,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1899,7 +1899,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -229.4,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1945,7 +1945,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -446.7,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -1991,7 +1991,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -830.0,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -2040,7 +2040,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -638.5,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -2133,7 +2133,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -466.8,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -2179,7 +2179,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -1344,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down Expand Up @@ -2228,7 +2228,7 @@
"enth_mol_form_liq_comp_ref": [
{
"v": -1072,
"u": "J/mol",
"u": "kJ/mol",
"i": 0
}
],
Expand Down
60 changes: 50 additions & 10 deletions watertap/examples/chemistry/chem_scaling_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@
from idaes.core.util import scaling as iscale
from pyomo.environ import value

__author__ = "Austin Ladshaw"
from idaes.models.properties.modular_properties.reactions.equilibrium_forms import (
power_law_equil,
solubility_product,
)

__author__ = "Austin Ladshaw, Xinhong Liu"

## Helper function for setting eps values associated with solubility_product functions
# NOTE: Function does nothing if no solubility reactions are present
def _set_eps_vals(rxn_params, rxn_config, factor=1e-2, max_k_eq_ref=1e-16):
def _set_eps_vals(rxn_params, rxn_config, factor=1e-2, max_k_eq_ref=1e-12):
# For solubility reactions, have to set the eps value
if hasattr(rxn_params, "equilibrium_reaction_idx"):
for rid in rxn_params.equilibrium_reaction_idx:
Expand All @@ -38,7 +43,7 @@ def _set_eps_vals(rxn_params, rxn_config, factor=1e-2, max_k_eq_ref=1e-16):


## Helper function for setting scaling factors for equilibrium reactions
def _set_equ_rxn_scaling(unit, rxn_config, min_k_eq_ref=1e-3):
def _set_equ_rxn_scaling(unit, rxn_params, rxn_config, min_k_eq_ref=1e-3):
# Add scaling factors for reactions (changes depending on if it is a log form or not)
for i in unit.control_volume.equilibrium_reaction_extent_index:
# i[0] = time, i[1] = reaction
Expand All @@ -50,9 +55,44 @@ def _set_equ_rxn_scaling(unit, rxn_config, min_k_eq_ref=1e-3):
iscale.set_scaling_factor(
unit.control_volume.equilibrium_reaction_extent[0.0, i[1]], 10 / scale
)

# Add scale for constraints in log form
log_scale = min(min_k_eq_ref, 1e-3)
# Scale keq calculation from keq_ref
iscale.constraint_scaling_transform(
unit.control_volume.reactions[0.0].equilibrium_constraint[i[1]], 0.1
)
unit.control_volume.reactions[0.0].log_k_eq_constraint[i[1]],
1 * scale / log_scale,
)

if (
rxn_config["equilibrium_reactions"][i[1]]["equilibrium_form"]
is solubility_product
or rxn_config["equilibrium_reactions"][i[1]]["equilibrium_form"]
is power_law_equil
):
if hasattr(rxn_params.component("reaction_" + i[1]), "eps"):
iscale.constraint_scaling_transform(
unit.control_volume.reactions[0.0].equilibrium_constraint[i[1]],
10 / scale,
)
else:
iscale.constraint_scaling_transform(
unit.control_volume.reactions[0.0].equilibrium_constraint[i[1]],
1 / scale,
)

else:
# Solubility_product with eps has different order of magnitude compare to other equilibrium constraints
if hasattr(rxn_params.component("reaction_" + i[1]), "eps"):
iscale.constraint_scaling_transform(
unit.control_volume.reactions[0.0].equilibrium_constraint[i[1]],
1 * scale / log_scale,
)
else:
iscale.constraint_scaling_transform(
unit.control_volume.reactions[0.0].equilibrium_constraint[i[1]],
0.1 * scale / log_scale,
)


## Helper function for setting scaling factors for inherent reactions
Expand Down Expand Up @@ -99,7 +139,7 @@ def _set_mat_bal_scaling_FpcTP(unit, min_flow_mol_phase_comp=1e-3):
unit.control_volume.properties_out[0.0].mole_frac_comp[i[1]], 10 / scale
)
iscale.set_scaling_factor(
unit.control_volume.properties_out[0.0].mole_frac_phase_comp[i], 10 / scale
unit.control_volume.properties_out[0.0].mole_frac_phase_comp[i], 100 / scale
)
iscale.set_scaling_factor(
unit.control_volume.properties_out[0.0].flow_mol_phase_comp[i], 10 / scale
Expand All @@ -124,7 +164,7 @@ def _set_mat_bal_scaling_FTPx(unit, min_mole_frac_comp=1e-3):
unit.control_volume.properties_out[0.0].mole_frac_comp[i[1]], 10 / scale
)
iscale.set_scaling_factor(
unit.control_volume.properties_out[0.0].mole_frac_phase_comp[i], 10 / scale
unit.control_volume.properties_out[0.0].mole_frac_phase_comp[i], 100 / scale
)
iscale.set_scaling_factor(
unit.control_volume.properties_out[0.0].flow_mol_phase_comp[i], 10 / scale
Expand Down Expand Up @@ -156,12 +196,12 @@ def _set_ene_bal_scaling(unit):
)
max_enth_mol_phase = max(val, max_enth_mol_phase)
iscale.set_scaling_factor(
unit.control_volume.properties_in[0.0]._enthalpy_flow_term[phase], 10 / val
unit.control_volume.properties_in[0.0]._enthalpy_flow_term[phase], 1 / val
)
iscale.set_scaling_factor(
unit.control_volume.properties_out[0.0]._enthalpy_flow_term[phase], 10 / val
unit.control_volume.properties_out[0.0]._enthalpy_flow_term[phase], 1 / val
)

iscale.constraint_scaling_transform(
unit.control_volume.enthalpy_balances[0.0], 10 / max_enth_mol_phase
unit.control_volume.enthalpy_balances[0.0], 1 / max_enth_mol_phase
)
2 changes: 1 addition & 1 deletion watertap/examples/chemistry/tests/test_chlorination.py
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ def test_scaling(self, chlorination_obj):
model = chlorination_obj

_set_inherent_rxn_scaling(model.fs.unit, thermo_config)
_set_equ_rxn_scaling(model.fs.unit, reaction_config)
_set_equ_rxn_scaling(model.fs.unit, model.fs.rxn_params, reaction_config)
_set_mat_bal_scaling_FTPx(model.fs.unit)
_set_ene_bal_scaling(model.fs.unit)

Expand Down
Loading