arviz-devs / arviz

Exploratory analysis of Bayesian models with Python
https://python.arviz.org
Apache License 2.0
1.59k stars 395 forks source link

Should there be a third option `InferenceData.extend(how="raise")`? #2331

Closed ricardoV94 closed 6 months ago

ricardoV94 commented 6 months ago

The default behavior extend(how="left") is a bit surprising https://github.com/pymc-devs/pymc/issues/7214

I wonder if the default should be to error or at least warn if a group already exists.

OriolAbril commented 6 months ago

.extend was designed precisely to behave like that, as opposed to concat. There are many situations (like posterior predictive sampling) where we already have an InferenceData with several groups, and we get a new one with repeated and new groups, and what we want to do is add the new ones to what we have, ignore the rest/assume they are the same.

Within sample_posterior_predictive for example, observed_data and constant_data will be present in both, so using how=raise or concat would fix that issue at the exepense of raising an error/warning every time the function is called. It is probably better to check if the posterior_predictive group is already present in the input and if so do one of: 1) default to predictions=True, 2) raise an error, 2) emit a warning and remove the existing posterior_predictive data.

Side note, I'll look into why the docstring is rendered so badly, which among other things makes concat be very hidden in the see also section.

ricardoV94 commented 6 months ago

Thanks @OriolAbril . I'll close this and just consider a misuse on the PyMC side