Skip to content

Commit

Permalink
Fix bug introduced in arviz.py and add regression test for #6496
Browse files Browse the repository at this point in the history
  • Loading branch information
covertg committed Mar 12, 2023
1 parent 7677f0c commit c535718
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 18 deletions.
16 changes: 10 additions & 6 deletions pymc/backends/arviz.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,19 @@ def __init__(
" one of trace, prior, posterior_predictive or predictions."
)

coords_typed = self.model.coords_typed
if coords:
coords_typed.update(_as_coord_vals(coords))
coords_typed = {
given_coords = coords if coords is not None else {}
given_coords_typed = {
cname: _as_coord_vals(cvals)
for cname, cvals in given_coords.items()
if cvals is not None
}
model_coords_typed = {
cname: cvals_typed
for cname, cvals_typed in coords_typed.items()
for cname, cvals_typed in self.model.coords_typed.items()
if cvals_typed is not None
}
self.coords = coords_typed
# Coords from argument should have precedence
self.coords = {**model_coords_typed, **given_coords_typed}

self.dims = {} if dims is None else dims
model_dims = {k: list(v) for k, v in self.model.named_vars_to_dims.items()}
Expand Down
5 changes: 2 additions & 3 deletions pymc/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,9 +1061,8 @@ def add_coord(
if values is not None:
# Conversion to numpy array to ensure coord vals are 1-dim
values = _as_coord_vals(values)
if name in self.coords_typed:
if not np.array_equal(_as_coord_vals(values), self.coords_typed[name]):
raise ValueError(f"Duplicate and incompatible coordinate: {name}.")
if name in self.coords_typed and not np.array_equal(values, self.coords_typed[name]):
raise ValueError(f"Duplicate and incompatible coordinate: {name}.")
if length is not None and not isinstance(length, (int, Variable)):
raise ValueError(
f"The `length` passed for the '{name}' coord must be an int, PyTensor Variable or None."
Expand Down
33 changes: 24 additions & 9 deletions tests/backends/test_arviz.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,15 +630,30 @@ def test_issue_5043_autoconvert_coord_values(self):
# We're not automatically converting things other than tuple,
# so advanced use cases remain supported at the InferenceData level.
# They just can't be used in the model construction already.
converter = InferenceDataConverter(
trace=mtrace,
coords={
"city": pd.MultiIndex.from_tuples(
[("Bonn", 53111), ("Berlin", 10178)], names=["name", "zipcode"]
)
},
)
assert isinstance(converter.coords["city"], pd.MultiIndex)
# TODO: we now ensure that everything passed to arviz is converted to
# ndarray, which makes the following test fail.
# converter = InferenceDataConverter(
# trace=mtrace,
# coords={
# "city": pd.MultiIndex.from_tuples(
# [("Bonn", 53111), ("Berlin", 10178)], names=["name", "zipcode"]
# )
# },
# )
# assert isinstance(converter.coords["city"], pd.MultiIndex)

def test_nested_coords_issue_6496(self):
"""Regression test to ensure we don't bug out if coordinate values
appear "nested" to numpy.
"""
model = pm.Model(coords={"cname": [("a", 1), ("a", 2), ("b", 1)]})
idata = to_inference_data(
prior={"x": np.zeros((100, 3))}, dims={"x": ["cname"]}, model=model
)
idata_coord = idata.prior.coords["cname"]
assert len(idata_coord) == 3
assert idata_coord.dtype == np.dtype("O")
assert np.array_equal(idata_coord.data, model.coords_typed["cname"])

def test_variable_dimension_name_collision(self):
with pytest.raises(ValueError, match="same name as its dimension"):
Expand Down

0 comments on commit c535718

Please sign in to comment.