-
-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added example for concat method #1037
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments.
InferenceData
should always be formatted the same way.
Also, I think that using from_dict instead of convert_to_inference_data will be more concise (no meed for chain and draw coordinates and xarray import won't be needed anymore).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you still use multidimensional data and a custom coord for the third dimension?
@OriolAbril Do I add more examples regarding these? I don't understand the question. |
In the first versions of the example, the inference data created had groups with several variables and each of these variables had multiple dimensions. Now it looks like the generated inference data has groups that contain only a scalar? |
Okay, I understand now. So, you're asking for something like this, right? (I don't think coords is necessary)
|
Yes, something like this |
I have used the same example as above. Should we also add examples for cases for concatenating over |
arviz/data/inference_data.py
Outdated
...: data = { | ||
...: "a": (["chain", "draw", "a_dim"], np.random.normal(size=(4, 100, 3))), | ||
...: "b": (["chain", "draw"], np.random.normal(size=(4, 100))), | ||
...: } | ||
...: coords = { | ||
...: "chain": (["chain"], np.arange(4)), | ||
...: "draw": (["draw"], np.arange(100)), | ||
...: "a_dim": (["a_dim"], ["x", "y", "z"]), | ||
...: } | ||
...: dataA = az.from_dict(posterior=data, coords=coords) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this code locally and it does not create an InferenceData object properly. ArviZ idiom should be followed:
data = {
"a": np.random.normal(size=(4, 100, 3)),
"b": np.random.normal(size=(4, 100))
}
coords = {"a_dim": ["x", "y", "z"]}
dataA = az.from_dict(data, coords=coords, dims={"a": ["a_dim"]})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, for the correction. It seems the xarray dataset (used in the previous case) map dims automatically, but we need to specify the mapping in case of from_dict
. Is this the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xarray's Dataset allows several notations, in the one used, the data and coordinates are specified as dicts of tuples, where the tuple contains both the data and the dimensions. ArviZ uses only one syntax, dicts of arrays for all 3 of them, the values of the dicts are directly understood as data, coordinate values or dimension list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks for the clarification :)
arviz/data/inference_data.py
Outdated
...: coords = { | ||
...: "a_dim": ["x", "y", "z"] | ||
...: } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be in the same line, it is not too long
arviz/data/inference_data.py
Outdated
...: "a": (["chain", "draw", "a_dim"], np.random.normal(size=(4, 100, 3))), | ||
...: "b": (["chain", "draw"], np.random.normal(size=(4, 100))), | ||
...: } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should follow black formatting, the }
should be on the same level as the d
in data and the two lines in between indented 4 spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Do you plan to add the concatenation over chain
or draw
here or in another PR? I don't want to merge before you have finished.
Hey @OriolAbril , Sorry for the delay. I was on the move. I'll add examples for chain and draw. |
Don't worry, there is no rush! |
Description
Fixes #552
Checklist
PR format?