-
Notifications
You must be signed in to change notification settings - Fork 0
CausalEstimand
class for evaluating causal estimands
#40
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
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
65d8936
to
a0c1df4
Compare
CausalEstimand
class for evaluating causal estimands
I did this exact same change yesterday when I wrote a new test in #42. Yet to think of a more elegant way to do it |
mscroggs
approved these changes
May 1, 2025
mscroggs
added a commit
that referenced
this pull request
May 1, 2025
* add parameter node * Update src/causalprog/graph/node.py * Update src/causalprog/graph/node.py * Update src/causalprog/graph/node.py * Update src/causalprog/graph/node.py * ruff * fix * fix test * need noqa here as they're somethimes used as kwargs * Move parameter functions and test from #40 * typing * docstring * Update src/causalprog/graph/node.py Co-authored-by: Will Graham <[email protected]> * Update src/causalprog/graph/node.py Co-authored-by: Will Graham <[email protected]> * rufffff * ruffffffff --------- Co-authored-by: Will Graham <[email protected]>
This was referenced May 1, 2025
Merged
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.
Partially addresses #37 | Target is #39 since
ParameterNode
s are needed to get this functionality working, but @mscroggs feel free to hold off the merge until you're happy with the base (since this will need rebasing after any updates to that branch, anyway).Introduces the
CausalProblem
class, designed to handle interactions between aGraph
(.graph
), the causal estimand of interest (.causal_estimand
) and the constraints (currently not implemented). Handing of the constraints is not introduced in this pull request, since any comments about how thecausal_estimand
is being handled would apply to the constraints too, so figured it was best to settle on a design for one before writing the next.Key Changes
Graph class
Had to introduce two methods related to
ParameterNode
s - mostly for fetching a tuple of all such nodes (note that this method usesgraph.ordered_nodes
since it is important that the parameter nodes a consistently given in the same order) and for the setting of the.value
attributes of these nodes.@mscroggs you might want to steal the corresponding changes & their tests and move them into the base PR (#39).
Node class
The
sample
method forParameterNode
s needs to have the same argument names as the super-class for the "up-the-graph" logic to work without an error, so have removed the_
prefixes and slapped on a# noqa
for now, until we find a more elegant solution.I also took the liberty of telling the linter that unused
*args
and**kwargs
are OK. Since we're going to have a lot of inherited and overwritten methods that may not utilise all the arguments in the signature of the super-class.CausalProblem
A CausalProblem can be initialised with just a label, and optionally providing a
Graph
that the instance will store. Said graph will define the relationships between the RVs that the problem considers, and can be set later via the.graph
attribute if so desired. Very limited functionality (as in, essentially all you can do is set the.graph
attribute) is available for aCausalProblem
instance without a set.graph
attribute.Otherwise, the primary way a user is expected to interact with a
CausalProblem
is to provide the causal estimand function via theset_causal_estimand
method. This takes some callable object (sigma
) which is assumed to be a function of some RVs, with the RVs to be associated toNode
s in the.graph
. At present, we also have to run a few "hacks" due to some design aspects we haven't got round to addressing:set_causal_estimand
allows one of the arguments tosigma
to be aGraph
object. This is because our currentalgorithms
module only has methods that support calls viaexpectation(graph, node_name, ...)
etc.sigma
(and the tests for this method) provides a staticrng_key
andsample_size
, whereas in the long run we probably want these set instance-wise across the wholeCausalProblem
instance.Once set, the
.causal_estimand
method can be called, which will evaluate the givensigma
function using the associatedNode
s in.graph
. This method takes ajax.Array
as input, which is assumed to be an array of the parameter values - the order of the values matches that ofCausalProblem.graph.parameter_nodes
(hence why it is important for this method to always return parameter nodes in the same order). It then calls.graph.set_parameters
with the appropriate values extracted from the input array, before runningsigma
and returning the result. This gets around the issue identified in #37 regarding fixing a single realisation of a RV by accident..causal_estimand
primarily exists to be sent into a solver / optimiser - I don't envision the user ever wanting to evaluate it manually, and they certainly should not need to evaluate it manually via a parameter vector. If a user does want to evaluate their causal estimand at a particular set of parameters, the recommended way is to run