bugfix for fit_laplace absent dims #609
Conversation
…ms on data containers and deterministics
ricardoV94
left a comment
ricardoV94
left a comment
There was a problem hiding this comment.
why are we forcing dims in variables that didn't have then originally?
| dim_shapes = initval.shape if initval is not None else batched_rv.shape.eval()[2:] | ||
| laplace_model.add_coords( | ||
| {name: np.arange(shape) for name, shape in zip(dims[2:], dim_shapes)} | ||
| {name: pt.arange(shape) for name, shape in zip(dims[2:], dim_shapes)} |
There was a problem hiding this comment.
pt.arange doesn't sound correct for dims
There was a problem hiding this comment.
I switched back to np.arange I noticed a speed-up during tests. Can you help me understand why pt.arange should not be used for dims? Is it because that type of execution happens outside of the model graph?
There was a problem hiding this comment.
coordinates should be concrete valid values for Arviz. pt.arange is not a valid coord type. It's probably undefined behavior
Hey @ricardoV94, I am not sure I understand what you mean here. I compared the returned |
pm.sample doesn't add dims to model variables. Those show up in the conversion to InferenceData by arviz |
…variables that did not have them originally
Thank you, @ricardoV94. I was not aware of that. I made a change so that we don't force any dims on variables that were not assigned any in the original model context. |
2 participants
updated dim_shape assignment logic in fit_laplace to handle absent dims on data containers and deterministics. I also added a test for that specific scenario.
closes #581