Add docs on justifying instruments in the IV approach

[NathanielF]

Just a draft PR for the moment.

Pretty happy with the example. Need to add some more write up and discuss if we want to add JAX/Numpyro as a dependency to the package.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[NathanielF]

For discussion @drbenvincent , @juanitorduz how do you think we should handle the Jax/Numpyro install. I've modified the IV class here to sample the PPC using the experimental JAX flag @jessegrabowski recomended, but i'm unsure if you want to make this a default.

[drbenvincent]

Just a quick comment based on an initial look on my phone... This seems to remove sampling from the prior altogether?

[NathanielF]

It did, but need not stay that way... Jessie's trick only seemed to work for posterior sampling, so prior sampling of mvNormal remains slow on prior checks.

However, I could keep the prior sampling but only sample the beta parameters which is fast.

Could also just default to only sample for the IV class and show how to do prior and posterior checks in the notebook after the original fit...

Options sort of depend on what you want to do with Jax and how integral it should be to CausalPy installation?

[jessegrabowski]

You can do it for prior as well, but you have to freeze the model before you sample, as in:

with pm.Model() as m:
    ...

from pymc.model.transform.optimization import freeze_dims_and_data

with freeze_dims_and_data(m):
    prior = pm.sample_prior_predictive(compile_kwargs={'mode':'JAX'})

I am going to open an issue for JAX forward sampling support, because it's really nice and all these hoops are silly.

[drbenvincent]

Failing remote tests will be fixed when #346 is merged

[drbenvincent]

Options sort of depend on what you want to do with Jax and how integral it should be to CausalPy installation?

What would be the main con's of adding that as a dependency?

[NathanielF]

I guess it's just heavier and @jessegrabowski 's trick to speed up the mvNormal ppc, just seems hacky and maybe things change down the road...

On the plus side you enable numpyro sampling which is great.

I think we should enable it but not bake the dependency into the model fit step. Instead, code defensively. Keep the IV fit method light but demonstrate fast ppc usage in the notebook docs....

[drbenvincent]

The preference is to stay light, but if we get nifty new functionality then I don't see a fundamental problem with adding dependencies.

So in your proposal you'd use a cell magic which conda installs numpyro so that it's just there locally. That should work.

It's also worth remembering that in the tests/doctests the IV ones are the slowest. If they can be sped up then that would be great. But that probably would require adding dependencies?

[drbenvincent]

PS if you update from main all the checks should pass now

[NathanielF]

Trying to play with the freeze dim approach for prior predictive sampling and getting a JAX not implmented error:

[codecov]

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.96%. Comparing base (23cbfd1) to head (9eb41f1).
Report is 2 commits behind head on main.

Files	Patch %	Lines
causalpy/pymc_models.py	83.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #345      +/-   ##
==========================================
- Coverage   79.98%   79.96%   -0.03%     
==========================================
  Files          21       21              
  Lines        1634     1642       +8     
==========================================
+ Hits         1307     1313       +6     
- Misses        327      329       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[NathanielF]

Think I'm in a bit of a catch 22 re: failing tests. Added new test fails in github action because we require Jax installed in the deployment environment. Remove thar function call and test coverage fails...

[NathanielF]

Added further discussion on why IV regression is particularly well-suited to bayesian inference methods

[NathanielF]

Thanks adjusted. Added link to wikipedia article on the credibility revolution. It's useful language to have.

---

View entire conversation on ReviewNB

[NathanielF]

Removed LATE from title and added glossary link

---

View entire conversation on ReviewNB

[NathanielF]

Hid code and expanded explanation

---

View entire conversation on ReviewNB

[NathanielF]

Hid code

---

View entire conversation on ReviewNB

[NathanielF]

Hid code and expanded explanation

---

View entire conversation on ReviewNB

[NathanielF]

Done.

---

View entire conversation on ReviewNB

[NathanielF]

Added more signposting

---

View entire conversation on ReviewNB

[NathanielF]

Checked headers

---

View entire conversation on ReviewNB

[NathanielF]

Yep. Added

---

View entire conversation on ReviewNB

[NathanielF]

Added a one-liner

---

View entire conversation on ReviewNB

[NathanielF]

yep, removed.

---

View entire conversation on ReviewNB

[NathanielF]

added!

---

View entire conversation on ReviewNB

[NathanielF]

Hid code

---

View entire conversation on ReviewNB

[NathanielF]

I wasn't sure about changing the DAG here. I'm not sure it adds more as it's still the NEAR variable just adding more species of college... I think it's fine without a specific DAG.

---

View entire conversation on ReviewNB

[NathanielF]

Added in the detail

---

View entire conversation on ReviewNB

[NathanielF]

Adjusted

---

View entire conversation on ReviewNB

[NathanielF]

Updated

---

View entire conversation on ReviewNB

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

drbenvincent commented on 2024-06-18T11:09:46Z
----------------------------------------------------------------

duplicated word "interest interest"

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

drbenvincent commented on 2024-06-18T11:09:47Z
----------------------------------------------------------------

Great. Can you also add in a link "replication crisis" as a specific term simply because I've just heard that much more than credibility revolution. Link: https://en.wikipedia.org/wiki/Replication_crisis

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

drbenvincent commented on 2024-06-18T11:09:48Z
----------------------------------------------------------------

Thanks for adding LATE to the glossary. Can you add in the glossary link to the first mention of LATE please?

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

drbenvincent commented on 2024-06-18T11:09:49Z
----------------------------------------------------------------

repetition of "at least"

[drbenvincent]

Really nice, much clearer I think. Just a few minor edits and it's ready to merge :)