-
Notifications
You must be signed in to change notification settings - Fork 6
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
438 add various interventions to interfacesipynb #493
438 add various interventions to interfacesipynb #493
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@sabinala , could you resolve conflicts and then tag me when this is ready for review again? Thanks! |
I get an error with this notebook at the dynamic parameter intervention (which explains why the notebook CI test fails), but it is successfully running the static interventions. See my comment about a fresh environment in #524. |
@sabinala , I fixed the notebook up to |
@SamWitty Thanks, trying that now |
Adding @anirban-chaudhuri as a reviewer to confirm that this did not revert previous changes to |
@SamWitty starting with a fresh environment did fix the errors I was getting before. Are you no longer getting an error for def make_var_threshold(var: str, threshold: torch.Tensor):
def var_threshold(time, state):
return state[var] - threshold
return var_threshold I get a File ~/anaconda3/lib/python3.10/site-packages/pyro/infer/predictive.py:55, in <dictcomp>(.0)
52 collected.append(trace)
53 else:
54 collected.append(
---> 55 {site: trace.nodes[site]["value"] for site in return_site_shapes}
56 )
58 if return_trace:
59 return collected
KeyError: 'parameter_intervention_value_p_cbeta_0' |
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.
The last part with optimize is the old version. We need to copy over the newer notebook implementation for optimize
from interfaces.ipynb
from main
.
…ventions-to-interfacesipynb Merging main into this branch
@SamWitty @anirban-chaudhuri This notebook is ready to go and includes the new |
Nevermind...cannot get the notebook test to pass :( |
@sabinala , unfortunately the CI does not return very informative error messages for the notebook tests. When I run the |
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 believe the CI issues are now addressed.
@sabinala , this issue looks slightly more nuanced. I'll figure it out. Please hold off on making more commits. |
Closes #438