-
Notifications
You must be signed in to change notification settings - Fork 0
Lagrangian respects effect handlers #94
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…multiplier values as separate args
0f98c3c to
8a4483e
Compare
mscroggs
approved these changes
Sep 3, 2025
Co-authored-by: Matthew Scroggs <matthew.w.scroggs@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #90 | Depends on #93 (merge conflicts are resolved).
Adds the infrastructure that we need to handle
CasualProblems where (subsets of) the constraints and causal estimand require changes to theGraphstructure (either through conditioning or thedooperator, for example) whilst others do not.For components (component = a constraint or the causal estimand) that require effect handlers to be overlaid on top of the
_underlying_graph.model, we will need to create a separatePredictiveinstance that these components can sample from, which is separate from thePredictiveinstance corresponding to the un-edited_underlying_graph.model. We have to do this each time we encounter a component that requires application of a new combination of handlers.As such, the best way for us to do this is to build all the
Predictiveinstances we need before we create the Lagrangian function. When we do this, we can also create a cheap mapping from the components of the problem to the (index of the) predictive model that they use. Then, evaluating the Lagrangian consists of sampling from each of the predictive models, and passing the appropriate samples (which we can index in the same way as the predictive models) into the appropriate components.HandlerToApplyis a convenience class, just to ease the means of determining if two handlers that are being applied are "the same"._CPComponent.can_use_same_modelmethod is introduced to determine if two components can use the same model (determined by whether the effects they require are identical).CausalProblem._associate_models_to_componentsnow handles the pre-building Lagrangian association discussed above.