From 0904052a8ff983561c6a3e445d11d42b9f2c01d6 Mon Sep 17 00:00:00 2001 From: sin-ha Date: Thu, 2 Jul 2020 17:37:13 +0530 Subject: [PATCH 01/15] Fixes indexing in SolutionArray --- interfaces/cython/cantera/composite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index b614bf67cc..932a5ae7c9 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -574,7 +574,7 @@ def __getitem__(self, index): def __getattr__(self, name): if name in self._extra: - return np.array(self._extra[name]) + return self._extra[name] else: raise AttributeError("'{}' object has no attribute '{}'".format( self.__class__.__name__, name)) From d3d33dc075d3c0714587f17e8c56df32e7524b5b Mon Sep 17 00:00:00 2001 From: sin-ha Date: Thu, 2 Jul 2020 17:37:44 +0530 Subject: [PATCH 02/15] Include initialisation of extra columns with any iterable --- interfaces/cython/cantera/composite.py | 45 ++++++++++++------- interfaces/cython/cantera/test/test_thermo.py | 30 +++++++++++++ 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 932a5ae7c9..1e960d432a 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -529,6 +529,10 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): reserved = self.__dir__() self._extra = OrderedDict() + + if isinstance(extra, str): + extra = [extra] + if isinstance(extra, dict): for name, v in extra.items(): if name in reserved: @@ -536,25 +540,36 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): "Unable to create extra column '{}': name is already " "used by SolutionArray objects.".format(name)) if not np.shape(v): - self._extra[name] = [v]*self._shape[0] - elif len(v) == self._shape[0]: - self._extra[name] = list(v) + self._extra[name] = np.array([v]).reshape(self._shape) + elif len(v) == self._shape or np.array(v).shape == self._shape: + self._extra[name] = np.array(v) else: raise ValueError("Unable to map extra SolutionArray" "input for named {!r}".format(name)) - elif extra and self._shape == (0,): + elif extra is not None: + try: + iterator = iter(extra) + except TypeError : + raise ValueError( + "Extra properties can be created by passing an iterable" + " of names for the properties, if the SolutionArray is not initially" + " empty. If you want to supply initial values for the properties, use" + " a dictionary whose keys are the names of the properties and values " + " are the initial values.") from None + for name in extra: - if name in reserved: - raise ValueError( - "Unable to create extra column '{}': name is already " - "used by SolutionArray objects.".format(name)) - self._extra[name] = [] + if isinstance(name, str): + if name in reserved: + raise ValueError( + "Unable to create extra column '{}': name is already " + "used by SolutionArray objects.".format(name)) + self._extra[name] = np.empty(self._shape) - elif extra: - raise ValueError("Initial values for extra properties must be " - "supplied in a dict if the SolutionArray is not " - "initially empty") + else: + raise ValueError( + "Unable to create extra column, passed value {}: " + "is not a string".format(name)) if meta is None: self._meta = {} @@ -609,7 +624,7 @@ def append(self, state=None, **kwargs): raise IndexError("Can only append to 1D SolutionArray") for name, value in self._extra.items(): - value.append(kwargs.pop(name)) + np.append(value, kwargs.pop(name)) if state is not None: self._phase.state = state @@ -655,7 +670,7 @@ def sort(self, col, reverse=False): indices = indices[::-1] self._states = [self._states[ix] for ix in indices] for k, v in self._extra.items(): - self._extra[k] = list(np.array(v)[indices]) + self._extra[k] = np.array(v)[indices] def equilibrate(self, *args, **kwargs): """ See `ThermoPhase.equilibrate` """ diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index b8471842b8..51b4858b23 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1734,6 +1734,36 @@ def test_extra(self): with self.assertRaises(ValueError): states = ct.SolutionArray(self.gas, extra=['creation_rates']) + states = ct.SolutionArray(self.gas, shape=(0, 0), extra=("prop1")) + self.assertEqual(states.prop1.shape, (0, 0)) + + def test_extra1(self): + states = ct.SolutionArray(self.gas, 7, extra={'prop': range(7)}) + array = np.array(range(7)) + self.assertArrayNear(states.prop, array) + states.prop[1] = -5 + states.prop[3:5] = [0, 1] + array_mod = np.array([0, -5, 2, 0, 1, 5, 6]) + self.assertArrayNear(states.prop, array_mod) + + def test_extra2(self): + states = ct.SolutionArray(self.gas, shape=(5, 9), extra=["prop1", "prop2"]) + self.assertEqual(states.prop1.shape, (5, 9)) + states1 = ct.SolutionArray(self.gas, shape=(0, 0, 0), extra="prop1") + self.assertEqual(states1.prop1.shape, (0, 0, 0)) + + def test_extra3(self): + states2 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra="prop1") + self.assertEqual(states2.prop1.shape, (2, 6, 9)) + + def test_extra4(self): + states3 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=("prop1", "prop2", "prop3")) + self.assertEqual(states3.prop3.shape, (2, 6, 9)) + + def test_extra5(self): + properties_array = np.array(["prop1", "prop2", "prop3"]) + states4 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=properties_array) + self.assertEqual(states4.prop2.shape, (2, 6, 9)) def test_append(self): states = ct.SolutionArray(self.gas, 5) From 1c5eb09585ad071dc7a488e9164f406f187f48b1 Mon Sep 17 00:00:00 2001 From: sin-ha Date: Thu, 2 Jul 2020 17:49:22 +0530 Subject: [PATCH 03/15] Remove redundant Numpy conversion --- interfaces/cython/cantera/composite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 1e960d432a..14bc3e2c00 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -670,7 +670,7 @@ def sort(self, col, reverse=False): indices = indices[::-1] self._states = [self._states[ix] for ix in indices] for k, v in self._extra.items(): - self._extra[k] = np.array(v)[indices] + self._extra[k] = v[indices] def equilibrate(self, *args, **kwargs): """ See `ThermoPhase.equilibrate` """ From b3809e599847dff9f391174e4d3288546c2b8a5b Mon Sep 17 00:00:00 2001 From: sin-ha Date: Thu, 2 Jul 2020 22:50:17 +0530 Subject: [PATCH 04/15] Minor formatting updates to SolutionArray --- interfaces/cython/cantera/composite.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 14bc3e2c00..1058d8a803 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -544,21 +544,21 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): elif len(v) == self._shape or np.array(v).shape == self._shape: self._extra[name] = np.array(v) else: - raise ValueError("Unable to map extra SolutionArray" + raise ValueError("Unable to map extra SolutionArray " "input for named {!r}".format(name)) elif extra is not None: try: - iterator = iter(extra) + iter_extra = iter(extra) except TypeError : raise ValueError( - "Extra properties can be created by passing an iterable" - " of names for the properties, if the SolutionArray is not initially" - " empty. If you want to supply initial values for the properties, use" - " a dictionary whose keys are the names of the properties and values " - " are the initial values.") from None + "Extra properties can be created by passing an iterable " + "of names for the properties, if the SolutionArray is not initially " + "empty. If you want to supply initial values for the properties, use " + "a dictionary whose keys are the names of the properties and values " + "are the initial values.") from None - for name in extra: + for name in iter_extra: if isinstance(name, str): if name in reserved: raise ValueError( @@ -567,7 +567,7 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): self._extra[name] = np.empty(self._shape) else: - raise ValueError( + raise TypeError( "Unable to create extra column, passed value {}: " "is not a string".format(name)) From d0bf6492bd6362281281e216df9ed919055dcb2b Mon Sep 17 00:00:00 2001 From: sin-ha Date: Sat, 4 Jul 2020 00:16:23 +0530 Subject: [PATCH 05/15] Fix append method for SolutionArray --- interfaces/cython/cantera/composite.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 1058d8a803..9625875447 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -624,7 +624,9 @@ def append(self, state=None, **kwargs): raise IndexError("Can only append to 1D SolutionArray") for name, value in self._extra.items(): - np.append(value, kwargs.pop(name)) + value = list(value) + value.append(kwargs.pop(name)) + value = np.array(value) if state is not None: self._phase.state = state From c4187f02e2f73a1e2bf8143997b9561cd03ca1eb Mon Sep 17 00:00:00 2001 From: sin-ha Date: Sat, 4 Jul 2020 00:32:11 +0530 Subject: [PATCH 06/15] Add sin-ha to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 58d1c89c2b..599256f18d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -44,6 +44,7 @@ Jeff Santner (@jsantner) Satyam Saxena (@CyberDrudge) Ingmar Schoegl (@ischoegl), Louisiana State University Santosh Shanbhogue (@santoshshanbhogue), Massachusetts Institute of Technology +Harsh Sinha (@sin-ha) David Sondak Raymond Speth (@speth), Massachusetts Institute of Technology Sergey Torokhov (@band-a-prend) From 11f0b9c0e2abb002b4134dc260d9404f514aa823 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:23:36 -0400 Subject: [PATCH 07/15] [Thermo] Format of KeyErrors in SolutionArray Fix the formatting of the two KeyErrors in SolutionArray.append(). The first one was missing the format method on the string. The second was raising a double-KeyError due to the way the except is handled. --- interfaces/cython/cantera/composite.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 9625875447..205ce076ca 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -634,15 +634,19 @@ def append(self, state=None, **kwargs): elif len(kwargs) == 1: attr, value = next(iter(kwargs.items())) if frozenset(attr) not in self._phase._full_states: - raise KeyError("{} does not specify a full thermodynamic state") + raise KeyError( + "'{}' does not specify a full thermodynamic state".format(attr) + ) setattr(self._phase, attr, value) else: try: attr = self._phase._full_states[frozenset(kwargs)] except KeyError: - raise KeyError("{} is not a valid combination of properties " - "for setting the thermodynamic state".format(tuple(kwargs))) + raise KeyError( + "{} is not a valid combination of properties for setting " + "the thermodynamic state".format(tuple(kwargs)) + ) from None setattr(self._phase, attr, [kwargs[a] for a in attr]) self._states.append(self._phase.state) From cd430428d928b9d2c9830f291b51c4e3acb41f6f Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:27:02 -0400 Subject: [PATCH 08/15] [Thermo] All extra kwargs must be in append When appending to a SolutionArray, any extra values must be specified as kwargs. If they aren't present, the array lengths would become out of sync, or there would be a KeyError on the kwargs dictionary. Also adds a test for the failure modes of SolutionArray.append(). --- interfaces/cython/cantera/composite.py | 12 ++++++---- interfaces/cython/cantera/test/test_thermo.py | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 205ce076ca..1ddc6cd052 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -623,10 +623,14 @@ def append(self, state=None, **kwargs): if len(self._shape) != 1: raise IndexError("Can only append to 1D SolutionArray") - for name, value in self._extra.items(): - value = list(value) - value.append(kwargs.pop(name)) - value = np.array(value) + # This check must go before we start appending to any arrays so that + # array lengths don't get out of sync. + missing_extra_kwargs = self._extra.keys() - kwargs.keys() + if missing_extra_kwargs: + raise TypeError( + "Missing keyword arguments for extra values: " + "'{}'".format(", ".join(missing_extra_kwargs)) + ) if state is not None: self._phase.state = state diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index 51b4858b23..aa0905555c 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1789,6 +1789,28 @@ def test_append(self): self.assertNear(states.P[-1], 1e4) self.assertNear(states.T[-1], 300) + def test_append_failures(self): + states = ct.SolutionArray(self.gas, 5, extra={"prop": "value"}) + states.TPX = np.linspace(500, 1000, 5), 2e5, 'H2:0.5, O2:0.4' + self.assertEqual(states._shape, (5,)) + + with self.assertRaisesRegex(TypeError, "Missing keyword arguments for extra"): + states.append(T=1100, P=3e5, X="AR:1.0") + # Failing to append a state shouldn't change the size + self.assertEqual(states._shape, (5,)) + + with self.assertRaisesRegex(KeyError, "does not specify"): + # I is not a valid property + states.append(TPI=(1100, 3e5, "AR:1.0"), prop="value2") + # Failing to append a state shouldn't change the size + self.assertEqual(states._shape, (5,)) + + with self.assertRaisesRegex(KeyError, "is not a valid"): + # I is not a valid property + states.append(T=1100, P=3e5, I="AR:1.0", prop="value2") + # Failing to append a state shouldn't change the size + self.assertEqual(states._shape, (5,)) + def test_purefluid(self): water = ct.Water() states = ct.SolutionArray(water, 5) From fb4557cc06d1644610acd7b91f95a8404f49c2e3 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:30:45 -0400 Subject: [PATCH 09/15] [Thermo] Append to SolutionArray all at once Move append of extra values to the end of SolutionArray.append(). If the append is done at the beginning of the function, the append can happen even if the state is invalid. This would cause the length of the arrays to become out of sync. --- interfaces/cython/cantera/composite.py | 16 ++++++++++++++++ interfaces/cython/cantera/test/test_thermo.py | 12 ++++++++++++ 2 files changed, 28 insertions(+) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 1ddc6cd052..4fc8605d1d 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -632,6 +632,16 @@ def append(self, state=None, **kwargs): "'{}'".format(", ".join(missing_extra_kwargs)) ) + # For the checks of the state below, the kwargs dictionary can + # only contain keywords that match properties of the state. Here + # we pop any kwargs that have to do with the extra items so they + # aren't included in that check. They are put into a temporary + # storage so that appending can be done at the end of the function + # all at once. + extra_temp = {} + for name in self._extra: + extra_temp[name] = kwargs.pop(name) + if state is not None: self._phase.state = state @@ -655,6 +665,12 @@ def append(self, state=None, **kwargs): self._states.append(self._phase.state) self._indices.append(len(self._indices)) + for name, value in self._extra.items(): + # Casting to a list before appending is ~5x faster than using + # np.append when appending a single item. + v = value.tolist() + v.append(extra_temp.pop(name)) + self._extra[name] = np.array(v) self._shape = (len(self._indices),) @property diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index aa0905555c..847f56824a 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1789,6 +1789,18 @@ def test_append(self): self.assertNear(states.P[-1], 1e4) self.assertNear(states.T[-1], 300) + def test_append_with_extra(self): + states = ct.SolutionArray(self.gas, 5, extra={"prop": "value"}) + states.TPX = np.linspace(500, 1000, 5), 2e5, 'H2:0.5, O2:0.4' + self.assertEqual(states._shape, (5,)) + states.append(T=1100, P=3e5, X="AR:1.0", prop="value2") + self.assertEqual(states.prop[-1], "value2") + self.assertEqual(states.prop.shape, (6,)) + states.append(T=1100, P=3e5, X="AR:1.0", prop=100) + # NumPy converts to the existing type of the array + self.assertEqual(states.prop[-1], "100") + self.assertEqual(states.prop.shape, (7,)) + def test_append_failures(self): states = ct.SolutionArray(self.gas, 5, extra={"prop": "value"}) states.TPX = np.linspace(500, 1000, 5), 2e5, 'H2:0.5, O2:0.4' From ce737760b010f25e1f9f2b0b78aaae0836e6fc4d Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:49:36 -0400 Subject: [PATCH 10/15] [Test] Rename assigning to slice test --- interfaces/cython/cantera/test/test_thermo.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index 847f56824a..352151e8e5 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1737,13 +1737,13 @@ def test_extra(self): states = ct.SolutionArray(self.gas, shape=(0, 0), extra=("prop1")) self.assertEqual(states.prop1.shape, (0, 0)) - def test_extra1(self): + def test_assign_to_slice(self): states = ct.SolutionArray(self.gas, 7, extra={'prop': range(7)}) - array = np.array(range(7)) + array = np.arange(7) self.assertArrayNear(states.prop, array) states.prop[1] = -5 states.prop[3:5] = [0, 1] - array_mod = np.array([0, -5, 2, 0, 1, 5, 6]) + array_mod = np.array([0, -5, 2, 0, 1, 5, 6]) self.assertArrayNear(states.prop, array_mod) def test_extra2(self): From 6e4181b1bfd4a0f226f7b3e1ebea900c2e6f5fd8 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:55:57 -0400 Subject: [PATCH 11/15] [Thermo] Fix ValueError if single value is passed If a single value (e.g., integer or float) is passed as the value for an extra column, the array with the single value cannot be reshaped. Use np.full() instead. Add/rename tests for creating extra items by dicts and iterables, along with tests of the failure conditions. --- interfaces/cython/cantera/composite.py | 7 ++- interfaces/cython/cantera/test/test_thermo.py | 44 ++++++++++++++++--- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 4fc8605d1d..a08d77a0a4 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -540,13 +540,12 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): "Unable to create extra column '{}': name is already " "used by SolutionArray objects.".format(name)) if not np.shape(v): - self._extra[name] = np.array([v]).reshape(self._shape) - elif len(v) == self._shape or np.array(v).shape == self._shape: + self._extra[name] = np.full(self._shape, v) + elif np.array(v).shape == self._shape: self._extra[name] = np.array(v) else: raise ValueError("Unable to map extra SolutionArray " - "input for named {!r}".format(name)) - + "input named {!r}".format(name)) elif extra is not None: try: iter_extra = iter(extra) diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index 352151e8e5..d68a0430f6 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1725,17 +1725,49 @@ def test_slicing_ndim(self): self.assertArrayNear(col3.T, 900*np.ones(2)) self.assertArrayNear(row2.T, 900*np.ones(5)) - def test_extra(self): + def test_extra_create_by_dict(self): extra = OrderedDict([('grid', np.arange(10)), ('velocity', np.random.rand(10))]) states = ct.SolutionArray(self.gas, 10, extra=extra) keys = list(states._extra.keys()) self.assertEqual(keys[0], 'grid') - - with self.assertRaises(ValueError): - states = ct.SolutionArray(self.gas, extra=['creation_rates']) - states = ct.SolutionArray(self.gas, shape=(0, 0), extra=("prop1")) - self.assertEqual(states.prop1.shape, (0, 0)) + self.assertArrayNear(states.grid, np.arange(10)) + + def test_extra_no_shape(self): + # The shape of the value for "prop" here is (), which is falsey + # and causes the use of np.full() + states = ct.SolutionArray(self.gas, 3, extra={"prop": 1}) + self.assertEqual(states.prop.shape, (3,)) + self.assertArrayNear(states.prop, np.array((1, 1, 1))) + + # Check a multidimensional SolutionArray + states = ct.SolutionArray(self.gas, (2, 2), extra={"prop": 3}) + self.assertEqual(states.prop.shape, (2, 2)) + self.assertArrayNear(states.prop, np.array(((3, 3), (3, 3)))) + + def test_extra_wrong_shape(self): + with self.assertRaisesRegex(ValueError, "Unable to map"): + ct.SolutionArray(self.gas, (3, 3), extra={"prop": np.arange(3)}) + + def test_extra_create_by_iterable(self): + states = ct.SolutionArray(self.gas, extra=("prop1")) + self.assertEqual(states.prop1.shape, (0,)) + + # An integer is not an iterable, and only bare strings are + # turned into iterables + with self.assertRaisesRegex(ValueError, "Extra properties"): + ct.SolutionArray(self.gas, extra=2) + + def test_extra_not_string(self): + with self.assertRaisesRegex(TypeError, "is not a string"): + ct.SolutionArray(self.gas, extra=[1]) + + def test_extra_reserved_names(self): + with self.assertRaisesRegex(ValueError, "name is already used"): + ct.SolutionArray(self.gas, extra=["creation_rates"]) + + with self.assertRaisesRegex(ValueError, "name is already used"): + ct.SolutionArray(self.gas, extra={"creation_rates": 0}) def test_assign_to_slice(self): states = ct.SolutionArray(self.gas, 7, extra={'prop': range(7)}) From c1f30c923d5c002f71eed8dd0298d38047f81f64 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:58:18 -0400 Subject: [PATCH 12/15] [Thermo] Clarify extra items from iterables Make sure that ndarrays are one-dimensional. Move check for bare string. Clarify error messages. Reduce indentation. Add tests for creating extra items from bare strings and ndarrays. --- interfaces/cython/cantera/composite.py | 32 +++++++++--------- interfaces/cython/cantera/test/test_thermo.py | 33 +++++++++---------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index a08d77a0a4..3d30080465 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -530,9 +530,6 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): self._extra = OrderedDict() - if isinstance(extra, str): - extra = [extra] - if isinstance(extra, dict): for name, v in extra.items(): if name in reserved: @@ -547,28 +544,31 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): raise ValueError("Unable to map extra SolutionArray " "input named {!r}".format(name)) elif extra is not None: + if isinstance(extra, np.ndarray): + extra = extra.flatten() + elif isinstance(extra, str): + extra = [extra] + try: iter_extra = iter(extra) except TypeError : raise ValueError( "Extra properties can be created by passing an iterable " - "of names for the properties, if the SolutionArray is not initially " - "empty. If you want to supply initial values for the properties, use " - "a dictionary whose keys are the names of the properties and values " - "are the initial values.") from None + "of names for the properties. If you want to supply initial " + "values for the properties, use a dictionary whose keys are " + "the names of the properties and values are the initial " + "values.") from None for name in iter_extra: - if isinstance(name, str): - if name in reserved: - raise ValueError( - "Unable to create extra column '{}': name is already " - "used by SolutionArray objects.".format(name)) - self._extra[name] = np.empty(self._shape) - - else: + if not isinstance(name, str): raise TypeError( - "Unable to create extra column, passed value {}: " + "Unable to create extra column, passed value '{!r}' " "is not a string".format(name)) + if name in reserved: + raise ValueError( + "Unable to create extra column '{}': name is already " + "used by SolutionArray objects.".format(name)) + self._extra[name] = np.empty(self._shape) if meta is None: self._meta = {} diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index d68a0430f6..ac3f63ec17 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1769,6 +1769,10 @@ def test_extra_reserved_names(self): with self.assertRaisesRegex(ValueError, "name is already used"): ct.SolutionArray(self.gas, extra={"creation_rates": 0}) + def test_extra_create_by_string(self): + states = ct.SolutionArray(self.gas, extra="prop") + self.assertEqual(states.prop.shape, (0,)) + def test_assign_to_slice(self): states = ct.SolutionArray(self.gas, 7, extra={'prop': range(7)}) array = np.arange(7) @@ -1778,24 +1782,17 @@ def test_assign_to_slice(self): array_mod = np.array([0, -5, 2, 0, 1, 5, 6]) self.assertArrayNear(states.prop, array_mod) - def test_extra2(self): - states = ct.SolutionArray(self.gas, shape=(5, 9), extra=["prop1", "prop2"]) - self.assertEqual(states.prop1.shape, (5, 9)) - states1 = ct.SolutionArray(self.gas, shape=(0, 0, 0), extra="prop1") - self.assertEqual(states1.prop1.shape, (0, 0, 0)) - - def test_extra3(self): - states2 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra="prop1") - self.assertEqual(states2.prop1.shape, (2, 6, 9)) - - def test_extra4(self): - states3 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=("prop1", "prop2", "prop3")) - self.assertEqual(states3.prop3.shape, (2, 6, 9)) - - def test_extra5(self): + def test_extra_create_by_ndarray(self): properties_array = np.array(["prop1", "prop2", "prop3"]) - states4 = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=properties_array) - self.assertEqual(states4.prop2.shape, (2, 6, 9)) + states = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=properties_array) + self.assertEqual(states.prop1.shape, (2, 6, 9)) + self.assertEqual(states.prop2.shape, (2, 6, 9)) + self.assertEqual(states.prop3.shape, (2, 6, 9)) + # Ensure that a 2-dimensional array is flattened + properties_array = np.array((["prop1"], ["prop2"])) + states = ct.SolutionArray(self.gas, extra=properties_array) + self.assertEqual(states.prop1.shape, (0,)) + self.assertEqual(states.prop2.shape, (0,)) def test_append(self): states = ct.SolutionArray(self.gas, 5) @@ -1815,7 +1812,7 @@ def test_append(self): self.gas.TPX = 300, 1e4, 'O2:0.5, AR:0.5' HPY = self.gas.HPY - self.gas.TPX = 1200, 5e5, 'O2:0.3, AR:0.7' # to make sure it gets changed + self.gas.TPX = 1200, 5e5, 'O2:0.3, AR:0.7' # to make sure it gets changed states.append(HPY=HPY) self.assertEqual(states.cp_mass.shape, (8,)) self.assertNear(states.P[-1], 1e4) From bb8ee9df45bf747d6dafe29284e8deb1ab36dc17 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Mon, 6 Jul 2020 13:58:33 -0400 Subject: [PATCH 13/15] [Thermo] Remove unused variables in SolutionArray --- interfaces/cython/cantera/composite.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 3d30080465..9c299eb53e 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -905,7 +905,6 @@ def collect_data(self, cols=None, tabular=False, threshold=0, species=None): if tabular and len(self._shape) != 1: raise AttributeError("Tabular output of collect_data only works " "for 1D SolutionArray") - out = OrderedDict() # Create default columns (including complete state information) if cols is None: @@ -929,7 +928,6 @@ def collect_data(self, cols=None, tabular=False, threshold=0, species=None): def split(c, d): """ Split attribute arrays into columns for tabular output """ - single_species = False # Determine labels for the items in the current group of columns if c in self._n_species: collabels = ['{}_{}'.format(c, s) for s in self.species_names] From 70ad28250167b245a2a7b2d0812157f1908a166c Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Wed, 15 Jul 2020 09:53:49 -0400 Subject: [PATCH 14/15] [CI] Bump Python 2 builder to Ubuntu 18.04 This change bumps the builder that uses the system Python 2 to run SCons and the system Python 3 for the Python interface to Ubuntu 18.04 from 16.04. The reason for this change is that Ubuntu 16.04 provides NumPy 1.11, which does not support flexible dtypes when creating arrays using .full(). We have decided to drop support for NumPy older than 1.12 for this reason. Numpy 1.12 was released in January 2017. --- .github/workflows/main.yml | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 8b060ca814..18d06d4ec3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -151,10 +151,10 @@ jobs: --exclude='/doxygen/xml' --delete --delete-excluded \ "${DOCS_OUTPUT_DIR}" ${RSYNC_USER}@${RSYNC_SERVER}:${RSYNC_DEST} - ubuntu-1604-py2: - # Ubuntu 16.04 using Python 2 to run SCons and the system Python 3 for the interface - name: Python 2 running SCons on Ubuntu 16.04 - runs-on: ubuntu-16.04 + ubuntu-1804-py2: + # Ubuntu 18.04 using Python 2 to run SCons and the system Python 3 for the interface + name: Python 2 running SCons on Ubuntu 18.04 + runs-on: ubuntu-18.04 steps: - uses: actions/checkout@v2 name: Checkout the repository @@ -163,12 +163,9 @@ jobs: - name: Install Apt dependencies run: | sudo apt-get install libboost-dev gfortran scons python3-numpy \ - python3-pip python3-setuptools libsundials-serial-dev liblapack-dev \ + python3-pip python3-setuptools python3-h5py python3-pandas \ + python3-ruamel.yaml cython libsundials-dev liblapack-dev \ libblas-dev - - name: Install Python dependencies - # Don't include Pandas here due to install errors - run: | - sudo -H /usr/bin/python3 -m pip install ruamel.yaml cython h5py - name: Build Cantera run: scons build python_cmd=/usr/bin/python3 blas_lapack_libs=lapack,blas -j2 - name: Test Cantera From 8d88fee0171fbcbf2760aaf0862690f0ce0e2d32 Mon Sep 17 00:00:00 2001 From: "Bryan W. Weber" Date: Fri, 21 Aug 2020 20:07:05 -0400 Subject: [PATCH 15/15] [SolutionArray] Disallow empty extra columns If the extra columns are passed to the SolutionArray, they must either have initial values or the SolutionArray must be empty. --- interfaces/cython/cantera/composite.py | 8 ++++++-- interfaces/cython/cantera/test/test_thermo.py | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/interfaces/cython/cantera/composite.py b/interfaces/cython/cantera/composite.py index 9c299eb53e..f1d463dea0 100644 --- a/interfaces/cython/cantera/composite.py +++ b/interfaces/cython/cantera/composite.py @@ -544,6 +544,10 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): raise ValueError("Unable to map extra SolutionArray " "input named {!r}".format(name)) elif extra is not None: + if self._shape != (0,): + raise ValueError("Initial values for extra properties must be " + "supplied in a dictionary if the SolutionArray " + "is not initially empty.") if isinstance(extra, np.ndarray): extra = extra.flatten() elif isinstance(extra, str): @@ -551,7 +555,7 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): try: iter_extra = iter(extra) - except TypeError : + except TypeError: raise ValueError( "Extra properties can be created by passing an iterable " "of names for the properties. If you want to supply initial " @@ -568,7 +572,7 @@ def __init__(self, phase, shape=(0,), states=None, extra=None, meta=None): raise ValueError( "Unable to create extra column '{}': name is already " "used by SolutionArray objects.".format(name)) - self._extra[name] = np.empty(self._shape) + self._extra[name] = np.empty(shape=(0,)) if meta is None: self._meta = {} diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py index ac3f63ec17..3be7b8d23e 100644 --- a/interfaces/cython/cantera/test/test_thermo.py +++ b/interfaces/cython/cantera/test/test_thermo.py @@ -1745,6 +1745,15 @@ def test_extra_no_shape(self): self.assertEqual(states.prop.shape, (2, 2)) self.assertArrayNear(states.prop, np.array(((3, 3), (3, 3)))) + def test_extra_not_empty(self): + """Test that a non-empty SolutionArray raises a ValueError if + initial values for properties are not supplied. + """ + with self.assertRaisesRegex(ValueError, "Initial values for extra properties"): + ct.SolutionArray(self.gas, 3, extra=["prop"]) + with self.assertRaisesRegex(ValueError, "Initial values for extra properties"): + ct.SolutionArray(self.gas, 3, extra=np.array(["prop", "prop2"])) + def test_extra_wrong_shape(self): with self.assertRaisesRegex(ValueError, "Unable to map"): ct.SolutionArray(self.gas, (3, 3), extra={"prop": np.arange(3)}) @@ -1784,10 +1793,10 @@ def test_assign_to_slice(self): def test_extra_create_by_ndarray(self): properties_array = np.array(["prop1", "prop2", "prop3"]) - states = ct.SolutionArray(self.gas, shape=(2, 6, 9), extra=properties_array) - self.assertEqual(states.prop1.shape, (2, 6, 9)) - self.assertEqual(states.prop2.shape, (2, 6, 9)) - self.assertEqual(states.prop3.shape, (2, 6, 9)) + states = ct.SolutionArray(self.gas, shape=(0,), extra=properties_array) + self.assertEqual(states.prop1.shape, (0,)) + self.assertEqual(states.prop2.shape, (0,)) + self.assertEqual(states.prop3.shape, (0,)) # Ensure that a 2-dimensional array is flattened properties_array = np.array((["prop1"], ["prop2"])) states = ct.SolutionArray(self.gas, extra=properties_array)