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

Add SCIP solver for testing #1282

Merged
merged 7 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 0 deletions .github/actions/setup-idaes/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,9 @@ runs:
echo '::group::Output of "idaes get-extensions" command'
idaes get-extensions --extra petsc --verbose
echo '::endgroup::'
- name: Install SCIP from AMPL
shell: bash -l {0}
run: |
echo '::group::Output of "pip install ampl_module_scip" command'
${{ inputs.install-command }} --index-url https://pypi.ampl.com ampl_module_scip
echo '::endgroup::'
38 changes: 38 additions & 0 deletions idaes/core/util/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@
__author__ = "Andrew Lee"


import os
from typing import Callable, Union

from pyomo.environ import Constraint, Set, units, Var
from pyomo.common.config import ConfigBlock
from pyomo.common import Executable
from pyomo.common.dependencies import attempt_import

from idaes.core import (
declare_process_block_class,
Expand Down Expand Up @@ -409,3 +414,36 @@
return MaterialFlowBasis.mass
else:
return MaterialFlowBasis.other


def _enable_scip_solver_for_testing(
name: str = "scip",
) -> Union[Callable[[], None], None]:
ampl_module_scip, is_available = attempt_import("ampl_module_scip")
if not is_available:
_log.warning(

Check warning on line 424 in idaes/core/util/testing.py

View check run for this annotation

Codecov / codecov/patch

idaes/core/util/testing.py#L424

Added line #L424 was not covered by tests
"ampl_module_scip must be installed to enable SCIP solver for testing"
)
return None

Check warning on line 427 in idaes/core/util/testing.py

View check run for this annotation

Codecov / codecov/patch

idaes/core/util/testing.py#L427

Added line #L427 was not covered by tests

new_path_entry = ampl_module_scip.bin_dir
# TODO prepending new_path_entry to PATH means that the "testing" SCIP will "shadow"
# existing executable directories already on PATH
# this behavior would match the (implied) semantics of this function, i.e.
# "enable SCIP solver used for testing (at the expense of other non-testing SCIP executables)"
os.environ["PATH"] = os.pathsep.join([new_path_entry, os.environ["PATH"]])
Executable.rehash()

def remove_from_path():
path_as_list = os.environ["PATH"].split(os.pathsep)

try:
path_as_list.remove(new_path_entry)
except ValueError:

Check warning on line 442 in idaes/core/util/testing.py

View check run for this annotation

Codecov / codecov/patch

idaes/core/util/testing.py#L442

Added line #L442 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a bad idea to add a test to cover this case to future-proof against any changes in how AMPL and/or Python handle their PATH entries / modifications in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it'd be nice to have the "undo" functionality tested. It's a bit tricky but I'll see what I can do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrmundt I've added a basic test for the PATH manipulation: 986ba84

# not in PATH
pass

Check warning on line 444 in idaes/core/util/testing.py

View check run for this annotation

Codecov / codecov/patch

idaes/core/util/testing.py#L444

Added line #L444 was not covered by tests
else:
os.environ["PATH"] = os.pathsep.join(path_as_list)
Executable.rehash()

return remove_from_path
45 changes: 27 additions & 18 deletions idaes/core/util/tests/test_model_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,24 @@
_write_report_section,
_collect_model_statistics,
)
from idaes.core.util.testing import _enable_scip_solver_for_testing


__author__ = "Alex Dowling, Douglas Allan, Andrew Lee"

solver_available = SolverFactory("scip").available()

@pytest.fixture(scope="module")
def scip_solver():
solver = SolverFactory("scip")
undo_changes = None

if not solver.available():
undo_changes = _enable_scip_solver_for_testing()
if not solver.available():
pytest.skip(reason="SCIP solver not available")
yield solver
if undo_changes is not None:
undo_changes()


@pytest.fixture
Expand Down Expand Up @@ -1790,27 +1804,26 @@ def test_identify_candidates(self, model):

@pytest.mark.solver
@pytest.mark.component
@pytest.mark.skipif(not solver_available, reason="SCIP is not available")
def test_solve_candidates_milp(self, model):
def test_solve_candidates_milp(self, model, scip_solver):
dh = DegeneracyHunter2(model)
dh._prepare_candidates_milp()
dh._solve_candidates_milp()

assert value(dh.candidates_milp.nu[0]) == pytest.approx(-1e-05, rel=1e-5)
assert value(dh.candidates_milp.nu[1]) == pytest.approx(1e-05, rel=1e-5)
assert value(dh.candidates_milp.nu[0]) == pytest.approx(1e-05, rel=1e-5)
assert value(dh.candidates_milp.nu[1]) == pytest.approx(-1e-05, rel=1e-5)

assert value(dh.candidates_milp.y_pos[0]) == pytest.approx(0, abs=1e-5)
assert value(dh.candidates_milp.y_pos[1]) == pytest.approx(1, rel=1e-5)
assert value(dh.candidates_milp.y_pos[1]) == pytest.approx(0, rel=1e-5)

assert value(dh.candidates_milp.y_neg[0]) == pytest.approx(-0, abs=1e-5)
assert value(dh.candidates_milp.y_neg[1]) == pytest.approx(-0, abs=1e-5)
assert value(dh.candidates_milp.y_neg[0]) == pytest.approx(0, abs=1e-5)
assert value(dh.candidates_milp.y_neg[1]) == pytest.approx(1, abs=1e-5)

assert value(dh.candidates_milp.abs_nu[0]) == pytest.approx(1e-05, rel=1e-5)
assert value(dh.candidates_milp.abs_nu[1]) == pytest.approx(1e-05, rel=1e-5)

assert dh.degenerate_set == {
model.con2: -1e-05,
model.con5: 1e-05,
model.con2: value(dh.candidates_milp.nu[0]),
model.con5: value(dh.candidates_milp.nu[1]),
}

@pytest.mark.unit
Expand Down Expand Up @@ -1840,8 +1853,7 @@ def test_solve_ids_milp(self, model):

@pytest.mark.solver
@pytest.mark.component
@pytest.mark.skipif(not solver_available, reason="SCIP is not available")
def test_solve_ids_milp(self, model):
def test_solve_ids_milp(self, model, scip_solver):
dh = DegeneracyHunter2(model)
dh._prepare_ids_milp()
ids_ = dh._solve_ids_milp(cons=model.con2)
Expand All @@ -1859,8 +1871,7 @@ def test_solve_ids_milp(self, model):

@pytest.mark.solver
@pytest.mark.component
@pytest.mark.skipif(not solver_available, reason="SCIP is not available")
def test_find_irreducible_degenerate_sets(self, model):
def test_find_irreducible_degenerate_sets(self, model, scip_solver):
dh = DegeneracyHunter2(model)
dh.find_irreducible_degenerate_sets()

Expand All @@ -1871,8 +1882,7 @@ def test_find_irreducible_degenerate_sets(self, model):

@pytest.mark.solver
@pytest.mark.component
@pytest.mark.skipif(not solver_available, reason="SCIP is not available")
def test_report_irreducible_degenerate_sets(self, model):
def test_report_irreducible_degenerate_sets(self, model, scip_solver):
stream = StringIO()

dh = DegeneracyHunter2(model)
Expand All @@ -1898,8 +1908,7 @@ def test_report_irreducible_degenerate_sets(self, model):

@pytest.mark.solver
@pytest.mark.component
@pytest.mark.skipif(not solver_available, reason="SCIP is not available")
def test_report_irreducible_degenerate_sets_none(self, model):
def test_report_irreducible_degenerate_sets_none(self, model, scip_solver):
stream = StringIO()

# Delete degenerate constraint
Expand Down
39 changes: 39 additions & 0 deletions idaes/core/util/tests/test_scip_for_testing.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, @lbianchi-lbl ! Can I note how funny this name is, though? test for testing ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noting, that was very much on purpose! I like to think of it as "Barber paradox for software engineers" 😅

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#################################################################################
# The Institute for the Design of Advanced Energy Systems Integrated Platform
# Framework (IDAES IP) was produced under the DOE Institute for the
# Design of Advanced Energy Systems (IDAES).
#
# Copyright (c) 2018-2023 by the software owners: The Regents of the
# University of California, through Lawrence Berkeley National Laboratory,
# National Technology & Engineering Solutions of Sandia, LLC, Carnegie Mellon
# University, West Virginia University Research Corporation, et al.
# All rights reserved. Please see the files COPYRIGHT.md and LICENSE.md
# for full copyright and license information.
#################################################################################
import os

import pytest
from pyomo.environ import SolverFactory

from idaes.core.util.testing import _enable_scip_solver_for_testing


ampl_module_scip = pytest.importorskip(
"ampl_module_scip", reason="'ampl_module_scip' not available"
)


@pytest.mark.unit
def test_path_manipulation():
path_before_enabling = str(os.environ["PATH"])

func_to_revert_changes = _enable_scip_solver_for_testing()
path_after_enabling = str(os.environ["PATH"])
sf = SolverFactory("scip")
assert len(path_after_enabling) > len(path_before_enabling)
assert str(ampl_module_scip.bin_dir) in str(sf.executable())
assert sf.available()

func_to_revert_changes()
path_after_reverting = str(os.environ["PATH"])
assert path_after_reverting == path_before_enabling
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def __getitem__(self, key):
# Put abstract (non-versioned) deps here.
# Concrete dependencies go in requirements[-dev].txt
install_requires=[
"pyomo>=6.6.2",
"pyomo @ git+https://github.com/IDAES/[email protected]",
"pint", # required to use Pyomo units
"networkx", # required to use Pyomo network
"numpy",
Expand Down
Loading