From f50c650835ab6eefdd7d7c3423c96c3dd51cfa2f Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Mon, 28 Sep 2020 14:14:35 -0400 Subject: [PATCH 1/6] Added check for unitless connections --- openmdao/core/group.py | 9 +++++--- openmdao/utils/tests/test_units.py | 37 +++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index 3b940a1c3f..98a307a8c4 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -24,7 +24,8 @@ shape_to_len from openmdao.utils.general_utils import ContainsAll, simple_warning, common_subpath, \ conditional_error, _is_slicer_op -from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit +from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit, \ + valid_units from openmdao.utils.mpi import MPI, check_mpi_exceptions import openmdao.utils.coloring as coloring_mod from openmdao.utils.array_utils import evenly_distrib_idxs @@ -1661,11 +1662,13 @@ def _setup_connections(self): in_units = allprocs_abs2meta_in[abs_in]['units'] if out_units: - if not in_units: + unit = _find_unit(out_units) + base_powers = any(i >= 1 for i in unit._powers) + if not in_units and base_powers: msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ f"is connected to input '{abs_in}' which has no units." simple_warning(msg) - elif not is_compatible(in_units, out_units): + elif base_powers and not is_compatible(in_units, out_units): msg = f"{self.msginfo}: Output units of '{out_units}' for '{abs_out}' " + \ f"are incompatible with input units of '{in_units}' for '{abs_in}'." if self._raise_connection_errors: diff --git a/openmdao/utils/tests/test_units.py b/openmdao/utils/tests/test_units.py index e8db78bd49..f88a4e6846 100644 --- a/openmdao/utils/tests/test_units.py +++ b/openmdao/utils/tests/test_units.py @@ -4,9 +4,10 @@ import unittest # from openmdao.utils.assert_utils import assert_near_equal +import openmdao.api as om from openmdao.utils.units import NumberDict, PhysicalUnit, _find_unit, import_library, \ add_unit, add_offset_unit, unit_conversion, get_conversion -from openmdao.utils.assert_utils import assert_warning +from openmdao.utils.assert_utils import assert_warning, assert_near_equal class TestNumberDict(unittest.TestCase): @@ -289,6 +290,40 @@ def test_add_unit(self): else: self.fail("Expecting Key Error") + def test_connect_unitless_to_none(self): + import warnings + p = om.Problem() + ivc = p.model.add_subsystem('indeps', om.IndepVarComp()) + ivc.add_output('x', val=5.0, units='1/s*s') + ivc.add_output('y', val=10.0, units='Hz*s') + p.model.add_subsystem('exec_comp', om.ExecComp('z = x + y', z={'units': None}, + x={'units': None}, y={'units': None})) + p.model.connect('indeps.x', 'exec_comp.x') + p.model.connect('indeps.y', 'exec_comp.y') + + with warnings.catch_warnings(): + warnings.simplefilter("error") + p.setup() + + p.run_model() + assert_near_equal(p.get_val('exec_comp.z'), 15.0) + + def test_promote_unitless_and_none(self): + import warnings + p = om.Problem() + ivc = p.model.add_subsystem('indeps', om.IndepVarComp(), promotes_outputs=['x', 'y']) + ivc.add_output('x', val=5.0, units='1/s*s') + ivc.add_output('y', val=10.0, units='Hz*s') + p.model.add_subsystem('exec_comp', om.ExecComp('z = x + y', z={'units': None}, + x={'units': None}, y={'units': None}), + promotes_inputs=['x', 'y']) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + p.setup() + + p.run_model() + assert_near_equal(p.get_val('exec_comp.z'), 15.0) if __name__ == "__main__": unittest.main() From de8617bafcd2fdec7a58d7d9c7bb9f1e085765b0 Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Mon, 28 Sep 2020 14:23:32 -0400 Subject: [PATCH 2/6] cleanup --- openmdao/core/group.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index 98a307a8c4..ab3eea67d7 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -24,8 +24,7 @@ shape_to_len from openmdao.utils.general_utils import ContainsAll, simple_warning, common_subpath, \ conditional_error, _is_slicer_op -from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit, \ - valid_units +from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit from openmdao.utils.mpi import MPI, check_mpi_exceptions import openmdao.utils.coloring as coloring_mod from openmdao.utils.array_utils import evenly_distrib_idxs From 1a7c1dd810b70b9301db1134c8b700d5267c13b5 Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Mon, 28 Sep 2020 15:15:07 -0400 Subject: [PATCH 3/6] Added code to handle unitless output for a exec input with units and test --- openmdao/core/group.py | 18 +++++++++++------- openmdao/utils/tests/test_units.py | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index ab3eea67d7..903ef91b51 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -1661,13 +1661,14 @@ def _setup_connections(self): in_units = allprocs_abs2meta_in[abs_in]['units'] if out_units: - unit = _find_unit(out_units) - base_powers = any(i >= 1 for i in unit._powers) - if not in_units and base_powers: + out_unit_meta = _find_unit(out_units) if out_units else None + out_unit_base_powers = any(i >= 1 for i in out_unit_meta._powers) + + if not in_units and out_unit_base_powers: msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ f"is connected to input '{abs_in}' which has no units." simple_warning(msg) - elif base_powers and not is_compatible(in_units, out_units): + elif out_unit_base_powers and not is_compatible(in_units, out_units): msg = f"{self.msginfo}: Output units of '{out_units}' for '{abs_out}' " + \ f"are incompatible with input units of '{in_units}' for '{abs_in}'." if self._raise_connection_errors: @@ -1675,9 +1676,12 @@ def _setup_connections(self): else: simple_warning(msg) elif in_units is not None: - msg = f"{self.msginfo}: Input '{abs_in}' with units of '{in_units}' is " + \ - f"connected to output '{abs_out}' which has no units." - simple_warning(msg) + in_unit_meta = _find_unit(in_units) if in_units else None + in_unit_base_powers = any(i >= 1 for i in in_unit_meta._powers) + if in_unit_base_powers: + msg = f"{self.msginfo}: Input '{abs_in}' with units of '{in_units}' is " + \ + f"connected to output '{abs_out}' which has no units." + simple_warning(msg) fail = False diff --git a/openmdao/utils/tests/test_units.py b/openmdao/utils/tests/test_units.py index f88a4e6846..24bde5067f 100644 --- a/openmdao/utils/tests/test_units.py +++ b/openmdao/utils/tests/test_units.py @@ -2,6 +2,7 @@ import os import unittest +import warnings # from openmdao.utils.assert_utils import assert_near_equal import openmdao.api as om @@ -309,7 +310,6 @@ def test_connect_unitless_to_none(self): assert_near_equal(p.get_val('exec_comp.z'), 15.0) def test_promote_unitless_and_none(self): - import warnings p = om.Problem() ivc = p.model.add_subsystem('indeps', om.IndepVarComp(), promotes_outputs=['x', 'y']) ivc.add_output('x', val=5.0, units='1/s*s') @@ -325,5 +325,22 @@ def test_promote_unitless_and_none(self): p.run_model() assert_near_equal(p.get_val('exec_comp.z'), 15.0) + def test_promote_unitless_ivc_to_exec_comp(self): + p = om.Problem() + ivc = p.model.add_subsystem('indeps', om.IndepVarComp()) + ivc.add_output('x', val=5.0, units=None) + ivc.add_output('y', val=10.0, units='Hz*s') + p.model.add_subsystem('exec_comp', om.ExecComp('z = x + y', z={'units': None}, + x={'units': '1/s*s'}, y={'units': None})) + p.model.connect('indeps.x', 'exec_comp.x') + p.model.connect('indeps.y', 'exec_comp.y') + + with warnings.catch_warnings(): + warnings.simplefilter("error") + p.setup() + + p.run_model() + assert_near_equal(p.get_val('exec_comp.z'), 15.0) + if __name__ == "__main__": unittest.main() From 372d684bcf9495a938a5762b85fd3d3d4800635d Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Tue, 29 Sep 2020 14:39:51 -0400 Subject: [PATCH 4/6] Created utility function to check units --- openmdao/core/group.py | 14 +++++--------- openmdao/utils/units.py | 3 +++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index 903ef91b51..ee6c4e9d93 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -24,7 +24,8 @@ shape_to_len from openmdao.utils.general_utils import ContainsAll, simple_warning, common_subpath, \ conditional_error, _is_slicer_op -from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit +from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit, \ + is_unitless from openmdao.utils.mpi import MPI, check_mpi_exceptions import openmdao.utils.coloring as coloring_mod from openmdao.utils.array_utils import evenly_distrib_idxs @@ -1661,14 +1662,11 @@ def _setup_connections(self): in_units = allprocs_abs2meta_in[abs_in]['units'] if out_units: - out_unit_meta = _find_unit(out_units) if out_units else None - out_unit_base_powers = any(i >= 1 for i in out_unit_meta._powers) - - if not in_units and out_unit_base_powers: + if not in_units and is_unitless(out_units): msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ f"is connected to input '{abs_in}' which has no units." simple_warning(msg) - elif out_unit_base_powers and not is_compatible(in_units, out_units): + elif is_unitless(out_units) and not is_compatible(in_units, out_units): msg = f"{self.msginfo}: Output units of '{out_units}' for '{abs_out}' " + \ f"are incompatible with input units of '{in_units}' for '{abs_in}'." if self._raise_connection_errors: @@ -1676,9 +1674,7 @@ def _setup_connections(self): else: simple_warning(msg) elif in_units is not None: - in_unit_meta = _find_unit(in_units) if in_units else None - in_unit_base_powers = any(i >= 1 for i in in_unit_meta._powers) - if in_unit_base_powers: + if is_unitless(in_units): msg = f"{self.msginfo}: Input '{abs_in}' with units of '{in_units}' is " + \ f"connected to output '{abs_out}' which has no units." simple_warning(msg) diff --git a/openmdao/utils/units.py b/openmdao/utils/units.py index 3dc8b5a0b9..2253ee219f 100644 --- a/openmdao/utils/units.py +++ b/openmdao/utils/units.py @@ -844,6 +844,9 @@ def _update_library(cfg): _UNIT_CACHE = {} +def is_unitless(units): + unit_meta = _find_unit(units) if units else None + return any(i >= 1 for i in unit_meta._powers) def _find_unit(unit): """ From 7bc787088099d064508263a6e8a61b5d4819c15c Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Tue, 29 Sep 2020 15:03:23 -0400 Subject: [PATCH 5/6] pep --- openmdao/core/group.py | 6 +++--- openmdao/utils/units.py | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index ee6c4e9d93..1bf7255974 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -1662,11 +1662,11 @@ def _setup_connections(self): in_units = allprocs_abs2meta_in[abs_in]['units'] if out_units: - if not in_units and is_unitless(out_units): + if not in_units and _is_unitless(out_units): msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ f"is connected to input '{abs_in}' which has no units." simple_warning(msg) - elif is_unitless(out_units) and not is_compatible(in_units, out_units): + elif _is_unitless(out_units) and not is_compatible(in_units, out_units): msg = f"{self.msginfo}: Output units of '{out_units}' for '{abs_out}' " + \ f"are incompatible with input units of '{in_units}' for '{abs_in}'." if self._raise_connection_errors: @@ -1674,7 +1674,7 @@ def _setup_connections(self): else: simple_warning(msg) elif in_units is not None: - if is_unitless(in_units): + if _is_unitless(in_units): msg = f"{self.msginfo}: Input '{abs_in}' with units of '{in_units}' is " + \ f"connected to output '{abs_out}' which has no units." simple_warning(msg) diff --git a/openmdao/utils/units.py b/openmdao/utils/units.py index 2253ee219f..9c3221086c 100644 --- a/openmdao/utils/units.py +++ b/openmdao/utils/units.py @@ -844,10 +844,12 @@ def _update_library(cfg): _UNIT_CACHE = {} -def is_unitless(units): + +def _is_unitless(units): unit_meta = _find_unit(units) if units else None return any(i >= 1 for i in unit_meta._powers) + def _find_unit(unit): """ Find unit helper function. From 8d45f85fbf876f5e9f8b2a97d48bedc279abbe34 Mon Sep 17 00:00:00 2001 From: Danny Kilkenny Date: Tue, 29 Sep 2020 16:22:17 -0400 Subject: [PATCH 6/6] Bret's comments --- openmdao/core/group.py | 13 +++++++------ openmdao/utils/tests/test_units.py | 16 ++++++++++++++++ openmdao/utils/units.py | 2 +- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/openmdao/core/group.py b/openmdao/core/group.py index 1bf7255974..d5611a96ed 100644 --- a/openmdao/core/group.py +++ b/openmdao/core/group.py @@ -25,7 +25,7 @@ from openmdao.utils.general_utils import ContainsAll, simple_warning, common_subpath, \ conditional_error, _is_slicer_op from openmdao.utils.units import is_compatible, unit_conversion, _has_val_mismatch, _find_unit, \ - is_unitless + _is_unitless from openmdao.utils.mpi import MPI, check_mpi_exceptions import openmdao.utils.coloring as coloring_mod from openmdao.utils.array_utils import evenly_distrib_idxs @@ -1662,11 +1662,12 @@ def _setup_connections(self): in_units = allprocs_abs2meta_in[abs_in]['units'] if out_units: - if not in_units and _is_unitless(out_units): - msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ - f"is connected to input '{abs_in}' which has no units." - simple_warning(msg) - elif _is_unitless(out_units) and not is_compatible(in_units, out_units): + if not in_units: + if _is_unitless(out_units): + msg = f"{self.msginfo}: Output '{abs_out}' with units of '{out_units}' " + \ + f"is connected to input '{abs_in}' which has no units." + simple_warning(msg) + elif not is_compatible(in_units, out_units): msg = f"{self.msginfo}: Output units of '{out_units}' for '{abs_out}' " + \ f"are incompatible with input units of '{in_units}' for '{abs_in}'." if self._raise_connection_errors: diff --git a/openmdao/utils/tests/test_units.py b/openmdao/utils/tests/test_units.py index 24bde5067f..87c0c5c7f9 100644 --- a/openmdao/utils/tests/test_units.py +++ b/openmdao/utils/tests/test_units.py @@ -342,5 +342,21 @@ def test_promote_unitless_ivc_to_exec_comp(self): p.run_model() assert_near_equal(p.get_val('exec_comp.z'), 15.0) + def test_incompatible(self): + p = om.Problem() + ivc = p.model.add_subsystem('indeps', om.IndepVarComp(), promotes_outputs=['x', 'y']) + ivc.add_output('x', val=5.0, units='1/s*s') + ivc.add_output('y', val=10.0, units='Hz*s') + p.model.add_subsystem('exec_comp', om.ExecComp('z = x + y', z={'units': None}, + x={'units': None}, y={'units': 'ft'}), + promotes_inputs=['x', 'y']) + + msg = ("Group (): Output units of 'Hz*s' for 'indeps.y' are incompatible with input " + "units of 'ft' for 'exec_comp.y'.") + + with self.assertRaises(RuntimeError) as cm: + p.setup() + self.assertEqual(str(cm.exception), msg) + if __name__ == "__main__": unittest.main() diff --git a/openmdao/utils/units.py b/openmdao/utils/units.py index 9c3221086c..c4a78d7b6e 100644 --- a/openmdao/utils/units.py +++ b/openmdao/utils/units.py @@ -847,7 +847,7 @@ def _update_library(cfg): def _is_unitless(units): unit_meta = _find_unit(units) if units else None - return any(i >= 1 for i in unit_meta._powers) + return not unit_meta.is_dimensionless() def _find_unit(unit):