Skip to content
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

Introduce sampling dependency for auxiliary nodes in SearchForExplanation #578

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rfl-urbaniak
Copy link
Collaborator

Search cases where antecedent and witness candidates overlap occur fairly naturally when we suspect a node to be a cause and we also want to allow it to be a witness when testing other nodes as antecedents (same run forward, different samples). Ideally, we should be able to do this in a single search.

Currently, however, witness and antecedent auxiliary nodes are sampled independently. This, among other things, means that if an antecedent candidate is not preempted (i.e. we intend to execute an antecedent intervention) the witness auxiliary node can still be one (stochastically), effectively preempting this antecedent intervention.

This is not the intended behavior. We know that if a node is considered as an antecedent, it should not be at the same time treated as a potential witness, and so the intended behavior should be that witness sampling is reserved for samples in which the antecedent intervention is not performed, effectively setting witness preemption case to 0 if the antecedent preemption case is 0. Not doing so not only leads to inefficient sampling, but also might skew the probability estimates if they involve conditioning on the antecedent intervention being performed but the user forgets to condition at the same time on that node not being a witness.

This PR allows for improved behavior by pulling case sampling outside of the handlers and passing the sampled and dependency-corrected cases forward to the handlers.

The API is backward compatible - if num_samples and sampling_dimension are not passed, we default to the old behavior, which is sufficient for the more limited search cases that we have been using so far.

@rfl-urbaniak rfl-urbaniak requested a review from eb8680 February 21, 2025 19:01
@rfl-urbaniak rfl-urbaniak added the status:awaiting review Awaiting response from reviewer label Feb 21, 2025
@rfl-urbaniak
Copy link
Collaborator Author

There might be still some unrelated linting issues with the dynamical module, which I think will be resolved independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:awaiting review Awaiting response from reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant