start updating schema doc #2086
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2086 +/- ##
=======================================
Coverage 89.96% 89.96%
=======================================
Files 119 119
Lines 12408 12408
=======================================
Hits 11163 11163
Misses 1245 1245 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| #### `unconstrained_posterior` | ||
| Samples from the posterior distribution p(theta_transformed|y) in the unconstrained (also called transformed) space. | ||
|
|
||
| Only variables that undergo a transformation for sampling should be present here. |
There was a problem hiding this comment.
This might be a problematic for Stan where it is easy to calculate the transformation but hard to make sure that transformation is not unity (or is comparing values against each other valid way to do this?)
There was a problem hiding this comment.
This is only relative to the samples, so comparing values against each other is perfeclty fine. Even in edgecases like a transformation that is the unity in most of the space but not everywhere, if all the samples are the same then the transformation is not doing anything, we don't need to save the samples in both places; and if we generate new posterior samples, we also need to generate the associated unconstrained samples both of which are generated by the PPLs what will know if and which transformations to do.
Maybe we can write it in a bit more clear phrasing. Not the good style for the doc, but in a conversational style it could be something like: I have the posterior samples and I want to have the unconstrained samples. What are the variables whose
samples I need to replace with different ones? -> those variables and samples are the ones that go in this unconstrained_sample group.
OriolAbril
requested review from
sethaxen and
ahartikainen
and removed request for
sethaxen and
ahartikainen
|
Sorry for the delay @OriolAbril ! I will review later today. |
|
I would like to merge this before the holidays if someone can review 🙏🏿 |
| * `inference_library`: the library used to run the inference. | ||
| * `inference_library_version`: version of the inference library used. | ||
|
|
||
| Metadata can be stored at the whole `InferenceData` level but also at group level when needed. |
There was a problem hiding this comment.
Do we have any recommendations yet as to which metadata fields belong in groups vs InferenceData?
There was a problem hiding this comment.
not yet, I'll open an issue for this
Co-authored-by: Seth Axen <seth.axen@gmail.com> Co-authored-by: Ari Hartikainen <ahartikainen@users.noreply.github.com>
Co-authored-by: Seth Axen <seth@sethaxen.com>
Description
The goal is to close #230, close #2048 and close #2064. So even though the fix for #230 has already been voted on #2056, I expect some discussion to happen, especially around the rest of the changes.
I have pushed the branch to arviz repo to make it easy for other people to make changes directly. cc @sethaxen
I am also making the changes here for now, which should get the most visibility, we can move the page to the arviz-project repo after merging.
Checklist
📚 Documentation preview 📚: https://arviz--2086.org.readthedocs.build/en/2086/