IDAES / idaes-pse

The IDAES Process Systems Engineering Framework
https://idaes-pse.readthedocs.io/
Other
216 stars 234 forks source link

AttributeError: 'NonNumericValue' object has no attribute 'is_expression_type' when certain tests are deselected #1348

Closed lbianchi-lbl closed 8 months ago

lbianchi-lbl commented 8 months ago

I've run into an error that seems to occur when I try to run only part of the IDAES test suite.

In the same environment (Ubuntu 20.04, Python 3.12.1, idaes-pse in dev mode from #1346), I get two different outcomes depending on the arguments passed to pytest:

$ pytest -m 'not integration' -k test_functions_unit
================================================================ test session starts =================================================================
platform linux -- Python 3.12.1, pytest-8.0.1, pluggy-1.4.0
rootdir: /home/ludo/lbl/idaes/idaes-pse
configfile: pytest.ini
plugins: cov-4.1.0, anyio-4.3.0
collected 7264 items / 7245 deselected / 4 skipped / 19 selected                                                                                     
The following modules are registered in the importorskipper plugin
 and will cause tests to be skipped if any of the registered modules is not found: 
- idaes.core.dmf:   ['traitlets', 'tinydb']
- idaes.tests.test_import:  ['colorama']
- idaes.core.surrogate.keras_surrogate: ['omlt']
- idaes.core.ui.fsvis.tests.test_fsvis: ['requests']
- idaes.core.ui.fsvis.tests.test_model_server:  ['requests']

idaes/models/properties/general_helmholtz/tests/test_functions_unit.py ................F..                                                     [100%]

====================================================================== FAILURES ======================================================================
_______________________________________________________________ test_plot_no_exception _______________________________________________________________

    @pytest.mark.unit
    @pytest.mark.skipif(not available(), reason="General Helmholtz not available")
    def test_plot_no_exception():
        m = pyo.ConcreteModel()
        m.hparam = HelmholtzParameterBlock(
            pure_component="r1234ze", amount_basis=AmountBasis.MASS
        )
>       m.hparam.ph_diagram(isotherms=True)

idaes/models/properties/general_helmholtz/tests/test_functions_unit.py:1802: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
idaes/models/properties/general_helmholtz/helmholtz_functions.py:2101: in ph_diagram
    dome = self.dome_data()
idaes/models/properties/general_helmholtz/helmholtz_functions.py:1852: in dome_data
    pyo.units.convert(
/opt/conda/envs/dev-idaes-py312-2/lib/python3.12/site-packages/pyomo/core/base/units_container.py:1310: in convert
    src_pint_unit = self._get_pint_units(src)
/opt/conda/envs/dev-idaes-py312-2/lib/python3.12/site-packages/pyomo/core/base/units_container.py:1210: in _get_pint_units
    return self._pintUnitExtractionVisitor.walk_expression(expr=expr)
/opt/conda/envs/dev-idaes-py312-2/lib/python3.12/site-packages/pyomo/core/expr/visitor.py:276: in walk_expression
    result = self._process_node(root, RECURSION_LIMIT)
/opt/conda/envs/dev-idaes-py312-2/lib/python3.12/site-packages/pyomo/core/expr/visitor.py:472: in _process_node_bx
    tmp = self.beforeChild(node, child, child_idx)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pyomo.core.base.units_container.PintUnitExtractionVisitor object at 0x7f59908d0560>
node = <pyomo.core.expr.numeric_expr.NPV_ExternalFunctionExpression object at 0x7f596b198af0>
child = <pyomo.core.expr.numvalue.NonNumericValue object at 0x7f596b1039d0>, child_idx = 1

    def beforeChild(self, node, child, child_idx):
        ctype = child.__class__
        if ctype in native_types or ctype in pyomo_constant_types:
            return False, self._pint_dimensionless

>       if child.is_expression_type():
E       AttributeError: 'NonNumericValue' object has no attribute 'is_expression_type'

/opt/conda/envs/dev-idaes-py312-2/lib/python3.12/site-packages/pyomo/core/base/units_container.py:908: AttributeError
=============================================================== slowest 100 durations ================================================================
0.63s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_plot_no_exception
0.28s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_expression_writer_sat
0.23s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_available2
0.09s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_expression_writer_mass
0.05s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_htpx_mole
0.05s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_expression_writer_mole
0.05s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_default_initializer
0.04s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_htpx_mass
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_h2o_transport
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_r134a_thermo
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_co2_transport
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_r134a_transport
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_propane_transport
0.03s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_r1234ze_transport
0.02s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_errors
0.02s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_initialize_param_block
0.02s call     idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_add_function

(40 durations < 0.005s hidden.  Use -vv to show these durations.)
============================================================== short test summary info ===============================================================
FAILED idaes/models/properties/general_helmholtz/tests/test_functions_unit.py::test_plot_no_exception - AttributeError: 'NonNumericValue' object has no attribute 'is_expression_type'
lbianchi-lbl commented 8 months ago

Some preliminary testing:

EDIT Running git bisect on Pyomo/pyomo between 6.7.0 and 6.7.1 (using git bisect run bash -c "cd ../idaes-pse/ && pytest -m 'not integration' -k test_functions_unit") points to Pyomo/pyomo@cfa6ff49:

cfa6ff49f5d20c07af63faaba0b0877e1e327cb5 is the first bad commit
commit cfa6ff49f5d20c07af63faaba0b0877e1e327cb5
Author: John Siirola <jsiirola@users.noreply.github.com>
Date:   Sat Dec 23 16:14:05 2023 -0700

    Defer resolution of numpy import

 pyomo/common/dependencies.py             |  3 ++-
 pyomo/common/env.py                      |  1 +
 pyomo/common/numeric_types.py            |  8 ++++++
 pyomo/core/base/indexed_component.py     | 12 +++++----
 pyomo/core/expr/numeric_expr.py          | 38 ++++++-----------------------
 pyomo/core/tests/unit/test_numpy_expr.py |  2 +-
 pyomo/core/tests/unit/test_numvalue.py   | 42 ++++++++++++++++++++++----------
 pyomo/environ/tests/test_environ.py      |  2 --
 8 files changed, 55 insertions(+), 53 deletions(-)

EDIT I've also attached a snapshot of list(sys.modules.keys()) when the AttributeError is raised obtained by running pytest with --pdb for that commit: sys-modules.json

jsiirola commented 8 months ago

OK - there is a lot going on here, but it boils down to a logic error in how ExternalFunctions handle unrecognized types. Pyomo 6.7.1 removed the automatic import / registration of numpy types (if numpy was present), which - while cutting the time to import pyomo in half - is exposing holes in the type detection routines. It passes when the whole test suite is run because some other test encounters numpy first and correctly handles the type registration.

There are several workarounds that are possible:

  1. IDAES could import numpy from pyomo.common.dependencies.numpy (this guarantees that the registration is triggered)
  2. IDAES could
    from pyomo.common.dependencies import numpy_available
    bool(numpy_available)

    (which triggers the type registrations)

  3. do nothing (as this doesn't directly affect thinkgs)

I am working on a PR to Pyomo to close this particular hole in the type detection logic.