From 4ab748cb684be344e1c8a624b4adfe7d39e79d6d Mon Sep 17 00:00:00 2001 From: Tim Maier Ubuntu Workstation Date: Fri, 13 May 2022 14:01:46 +0200 Subject: [PATCH 1/6] check for original coords --- pymc/model.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pymc/model.py b/pymc/model.py index 47ecabc732..44c269fe83 100644 --- a/pymc/model.py +++ b/pymc/model.py @@ -1179,16 +1179,17 @@ def set_data( # Reject resizing if we already know that it would create shape problems. # NOTE: If there are multiple pm.MutableData containers sharing this dim, but the user only # changes the values for one of them, they will run into shape problems nonetheless. - length_belongs_to = length_tensor.owner.inputs[0].owner.inputs[0] - if not isinstance(length_belongs_to, SharedVariable) and length_changed: - raise ShapeError( - f"Resizing dimension '{dname}' with values of length {new_length} would lead to incompatibilities, " - f"because the dimension was initialized from '{length_belongs_to}' which is not a shared variable. " - f"Check if the dimension was defined implicitly before the shared variable '{name}' was created, " - f"for example by a model variable.", - actual=new_length, - expected=old_length, - ) + if original_coords is None: + length_belongs_to = length_tensor.owner.inputs[0].owner.inputs[0] + if not isinstance(length_belongs_to, SharedVariable) and length_changed: + raise ShapeError( + f"Resizing dimension '{dname}' with values of length {new_length} would lead to incompatibilities, " + f"because the dimension was initialized from '{length_belongs_to}' which is not a shared variable. " + f"Check if the dimension was defined implicitly before the shared variable '{name}' was created, " + f"for example by a model variable.", + actual=new_length, + expected=old_length, + ) if original_coords is not None and length_changed: if length_changed and new_coords is None: raise ValueError( From 5f4e5aa78fe938746a607fa9b13588bd47cdd33a Mon Sep 17 00:00:00 2001 From: Tim Maier Ubuntu Workstation Date: Mon, 16 May 2022 11:03:12 +0200 Subject: [PATCH 2/6] support TensorConstant as dim length in set_data --- pymc/model.py | 63 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/pymc/model.py b/pymc/model.py index 44c269fe83..7ad250885b 100644 --- a/pymc/model.py +++ b/pymc/model.py @@ -46,7 +46,7 @@ from aesara.tensor.random.opt import local_subtensor_rv_lift from aesara.tensor.random.var import RandomStateSharedVariable from aesara.tensor.sharedvar import ScalarSharedVariable -from aesara.tensor.var import TensorVariable +from aesara.tensor.var import TensorVariable, TensorConstant from pymc.aesaraf import ( compile_pymc, @@ -61,7 +61,7 @@ from pymc.distributions import joint_logpt from pymc.distributions.logprob import _get_scaling from pymc.distributions.transforms import _default_transform -from pymc.exceptions import ImputationWarning, SamplingError, ShapeError +from pymc.exceptions import ImputationWarning, ShapeWarning, SamplingError, ShapeError from pymc.initial_point import make_initial_point_fn from pymc.math import flatten_list from pymc.util import ( @@ -1179,24 +1179,49 @@ def set_data( # Reject resizing if we already know that it would create shape problems. # NOTE: If there are multiple pm.MutableData containers sharing this dim, but the user only # changes the values for one of them, they will run into shape problems nonetheless. - if original_coords is None: - length_belongs_to = length_tensor.owner.inputs[0].owner.inputs[0] - if not isinstance(length_belongs_to, SharedVariable) and length_changed: + if length_changed: + if isinstance(length_tensor,TensorConstant): raise ShapeError( - f"Resizing dimension '{dname}' with values of length {new_length} would lead to incompatibilities, " - f"because the dimension was initialized from '{length_belongs_to}' which is not a shared variable. " - f"Check if the dimension was defined implicitly before the shared variable '{name}' was created, " - f"for example by a model variable.", - actual=new_length, - expected=old_length, + f"Resizing dimension '{dname}' is impossible, because " + f"a 'TensorConstant' stores its length. To be able " + f"to change the dimension length, 'fixed' in " + f"'model.add_coord' must be passed False." ) - if original_coords is not None and length_changed: - if length_changed and new_coords is None: - raise ValueError( - f"The '{name}' variable already had {len(original_coords)} coord values defined for" - f"its {dname} dimension. With the new values this dimension changes to length " - f"{new_length}, so new coord values for the {dname} dimension are required." + if length_tensor.owner is None: + # This is the case if the dimension was initialized + # from custom coords, but dimension length was not + # stored in TensorConstant e.g by 'fixed' set to False + + warnings.warn( + f"You're changing the shape of a shared variable " + f"in the '{dname}' dimension which was initialized " + f"from coords. Make sure to update the corresponding " + f"coords, otherwise you'll get shape issues.", + ShapeWarning, ) + else: + length_belongs_to = length_tensor.owner.inputs[0].owner.inputs[0] + if not isinstance(length_belongs_to, SharedVariable): + raise ShapeError( + f"Resizing dimension '{dname}' with values of length {new_length} would lead to incompatibilities, " + f"because the dimension was initialized from '{length_belongs_to}' which is not a shared variable. " + f"Check if the dimension was defined implicitly before the shared variable '{name}' was created, " + f"for example by a model variable.", + actual=new_length, + expected=old_length, + ) + if original_coords is not None: + if new_coords is None: + raise ValueError( + f"The '{name}' variable already had {len(original_coords)} coord values defined for" + f"its {dname} dimension. With the new values this dimension changes to length " + f"{new_length}, so new coord values for the {dname} dimension are required." + ) + if isinstance(length_tensor, ScalarSharedVariable): + # Updating the shared variable resizes dependent nodes that use this dimension for their `size`. + length_tensor.set_value(new_length) + + if new_coords is not None: # Update the registered coord values (also if they were None) if len(new_coords) != new_length: @@ -1206,9 +1231,7 @@ def set_data( expected=new_length, ) self._coords[dname] = new_coords - if isinstance(length_tensor, ScalarSharedVariable) and new_length != old_length: - # Updating the shared variable resizes dependent nodes that use this dimension for their `size`. - length_tensor.set_value(new_length) + shared_object.set_value(values) From 8e2ff0a7be4edeac1a9fe0831383b306e0b9b09e Mon Sep 17 00:00:00 2001 From: Tim Maier Ubuntu Workstation Date: Mon, 16 May 2022 11:06:57 +0200 Subject: [PATCH 3/6] forgot pre-commit --- pymc/model.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pymc/model.py b/pymc/model.py index 7ad250885b..d3d224af77 100644 --- a/pymc/model.py +++ b/pymc/model.py @@ -46,7 +46,7 @@ from aesara.tensor.random.opt import local_subtensor_rv_lift from aesara.tensor.random.var import RandomStateSharedVariable from aesara.tensor.sharedvar import ScalarSharedVariable -from aesara.tensor.var import TensorVariable, TensorConstant +from aesara.tensor.var import TensorConstant, TensorVariable from pymc.aesaraf import ( compile_pymc, @@ -61,7 +61,7 @@ from pymc.distributions import joint_logpt from pymc.distributions.logprob import _get_scaling from pymc.distributions.transforms import _default_transform -from pymc.exceptions import ImputationWarning, ShapeWarning, SamplingError, ShapeError +from pymc.exceptions import ImputationWarning, SamplingError, ShapeError, ShapeWarning from pymc.initial_point import make_initial_point_fn from pymc.math import flatten_list from pymc.util import ( @@ -1180,7 +1180,7 @@ def set_data( # NOTE: If there are multiple pm.MutableData containers sharing this dim, but the user only # changes the values for one of them, they will run into shape problems nonetheless. if length_changed: - if isinstance(length_tensor,TensorConstant): + if isinstance(length_tensor, TensorConstant): raise ShapeError( f"Resizing dimension '{dname}' is impossible, because " f"a 'TensorConstant' stores its length. To be able " @@ -1221,7 +1221,6 @@ def set_data( # Updating the shared variable resizes dependent nodes that use this dimension for their `size`. length_tensor.set_value(new_length) - if new_coords is not None: # Update the registered coord values (also if they were None) if len(new_coords) != new_length: @@ -1232,7 +1231,6 @@ def set_data( ) self._coords[dname] = new_coords - shared_object.set_value(values) def register_rv( From 130671f86d94b2ae78d2bcc60d41bc173a3728a9 Mon Sep 17 00:00:00 2001 From: Tim Maier Ubuntu Workstation Date: Mon, 16 May 2022 16:38:00 +0200 Subject: [PATCH 4/6] added test --- pymc/model.py | 3 ++- pymc/tests/test_data_container.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pymc/model.py b/pymc/model.py index d3d224af77..a49c628870 100644 --- a/pymc/model.py +++ b/pymc/model.py @@ -1229,7 +1229,8 @@ def set_data( actual=len(new_coords), expected=new_length, ) - self._coords[dname] = new_coords + # store it as tuple for immutability as in add_coord + self._coords[dname] = tuple(new_coords) shared_object.set_value(values) diff --git a/pymc/tests/test_data_container.py b/pymc/tests/test_data_container.py index 2bcceaf5d7..3e86c726fa 100644 --- a/pymc/tests/test_data_container.py +++ b/pymc/tests/test_data_container.py @@ -316,7 +316,10 @@ def test_explicit_coords(self): # pass coordinates explicitly, use numpy array in Data container with pm.Model(coords=coords) as pmodel: pm.MutableData("observations", data, dims=("rows", "columns")) - + # new data with same shape + pm.set_data({"observations":data+1}) + # new data with same shape and coords + pm.set_data({"observations":data},coords=coords) assert "rows" in pmodel.coords assert pmodel.coords["rows"] == ("R1", "R2", "R3", "R4", "R5") assert "rows" in pmodel.dim_lengths From 5386ae9dbb6fa3515dc873e3646d521a5fc3477c Mon Sep 17 00:00:00 2001 From: Tim Maier Ubuntu Workstation Date: Mon, 16 May 2022 16:48:35 +0200 Subject: [PATCH 5/6] pre-commit --- pymc/tests/test_data_container.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pymc/tests/test_data_container.py b/pymc/tests/test_data_container.py index 3e86c726fa..6f9636cf37 100644 --- a/pymc/tests/test_data_container.py +++ b/pymc/tests/test_data_container.py @@ -317,9 +317,9 @@ def test_explicit_coords(self): with pm.Model(coords=coords) as pmodel: pm.MutableData("observations", data, dims=("rows", "columns")) # new data with same shape - pm.set_data({"observations":data+1}) + pm.set_data({"observations": data + 1}) # new data with same shape and coords - pm.set_data({"observations":data},coords=coords) + pm.set_data({"observations": data}, coords=coords) assert "rows" in pmodel.coords assert pmodel.coords["rows"] == ("R1", "R2", "R3", "R4", "R5") assert "rows" in pmodel.dim_lengths From a3dd51991012b3373d474a0ac54c61f17b0365ec Mon Sep 17 00:00:00 2001 From: Michael Osthege Date: Mon, 16 May 2022 19:29:59 +0200 Subject: [PATCH 6/6] Rephrase and typo fixes in error/warning messages --- pymc/model.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pymc/model.py b/pymc/model.py index a49c628870..a3c20bb81c 100644 --- a/pymc/model.py +++ b/pymc/model.py @@ -1185,7 +1185,7 @@ def set_data( f"Resizing dimension '{dname}' is impossible, because " f"a 'TensorConstant' stores its length. To be able " f"to change the dimension length, 'fixed' in " - f"'model.add_coord' must be passed False." + f"'model.add_coord' must be set to `False`." ) if length_tensor.owner is None: # This is the case if the dimension was initialized @@ -1193,7 +1193,7 @@ def set_data( # stored in TensorConstant e.g by 'fixed' set to False warnings.warn( - f"You're changing the shape of a shared variable " + f"You're changing the shape of a variable " f"in the '{dname}' dimension which was initialized " f"from coords. Make sure to update the corresponding " f"coords, otherwise you'll get shape issues.", @@ -1206,14 +1206,14 @@ def set_data( f"Resizing dimension '{dname}' with values of length {new_length} would lead to incompatibilities, " f"because the dimension was initialized from '{length_belongs_to}' which is not a shared variable. " f"Check if the dimension was defined implicitly before the shared variable '{name}' was created, " - f"for example by a model variable.", + f"for example by another model variable.", actual=new_length, expected=old_length, ) if original_coords is not None: if new_coords is None: raise ValueError( - f"The '{name}' variable already had {len(original_coords)} coord values defined for" + f"The '{name}' variable already had {len(original_coords)} coord values defined for " f"its {dname} dimension. With the new values this dimension changes to length " f"{new_length}, so new coord values for the {dname} dimension are required." )