-
-
Notifications
You must be signed in to change notification settings - Fork 21
Add support for multiple BART random variables per model. #231
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
base: main
Are you sure you want to change the base?
Conversation
Thanks, this is great! Adding one example will be fantastic! |
Before approving, I tested locally with a couple of examples, and they ran. But the test is failing. Funny that the order of the variables in the model affects the result. This works sigma = pm.HalfNormal("sigma", 1)
mu1 = pmb.BART("mu1", X1, Y1, m=5)
mu2 = pmb.BART("mu2", X2, Y2, m=5) but this fails mu1 = pmb.BART("mu1", X1, Y1, m=5)
mu2 = pmb.BART("mu2", X2, Y2, m=5)
sigma = pm.HalfNormal("sigma", 1) |
@@ -346,7 +364,7 @@ def resample( | |||
new_particles.append(particles[idx].copy()) | |||
else: | |||
new_particles.append(particles[idx]) | |||
seen.append(idx) | |||
seen.append(int(idx)) |
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.
Is this necessary?
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.
Nope! Sorry about that, unnecessary since it's already int
Hmm that's odd---I can't reproduce this behavior. I tried swapping the order of NUTS and pgbart variables in my tests and they still passed, and in my example notebook I'm working on and it ran. Or are you saying that it runs but the results are just different? To some extent I'd expect that with the sequential nature of the sampling steps but ideally they wouldn't be too different. If you can share reprex I can look into it? |
Locally I am getting the same error than here, it does not run unless the bart RVs are defined last. I am away from my laptop most likely until Monday. Are you using the last pymc version? |
Oh, looks like I am on pymc = 5.19.1 (I used environment-dev.yml) |
Alright I am able to reproduce this issue with For now, I suppose I could:
I'm afraid that diagnosing what has changed on the |
This PR adds new functionality to support multiple BART RVs defined within a single
pymc
model, addressing and resolve the limitation discussed in #86.pgbart
sampler with combined sampling handled bypymc
's compound sampling functionality)Example use
This addition allows for some new modeling possibilities. One example is the Bayesian Causal Forests model of Hahn et al.. This model aims to capture heterogenous treatment effects of a binary treatment$Z$ by separately capturing the prognostic predictiveness of those covariates $X$ and their interactions with $z$ (note that different $X$ may also be modeled by $\mu$ and $\tau$ ).
This can be implemented as:
Hope this looks good. I also plan to offer a more detailed demo example for the
pymc-examples
doc repo if desired!