search

BasisResearch / chirho

An experimental language for causal reasoning
https://basisresearch.github.io/chirho/getting_started.html
Apache License 2.0
172 stars 12 forks source link

Reuse non-default names across multiple splits in MultiWorldCounterfactual #528

Closed eb8680 closed 7 months ago

eb8680 commented 9 months ago

This PR changes the behavior of the MultiWorldCounterfactual handler. Previously, MultiWorldCounterfactual created a fresh index variable for every split() call, regardless of the name keyword argument passed to split. After this PR, MultiWorldCounterfactual only introduces fresh variables for fresh (or empty) names.

This change is necessary to support a more sophisticated counterfactual semantics in dynamical systems.

SamWitty commented 9 months ago

@eb8680 , once this lands I'd like to add a tiny PR that adds an optional name kwarg to the existing dynamical systems interventions, which should change the counterfactual behavior from "one world per state variable" to "one world per static/dynamic intervention". That will also hopefully make it much easier to index into worlds produced from dynamical systems interventions. I know this PR is already intended to lead to interventions on dynamics, but figured it would be worth mentioning how it will improve the ergonomics (with a minor change) of the existing dynamical systems interventions. Thoughts?

eb8680 commented 8 months ago

I guess this is ready for review, but since it's a breaking change I would be hesitant to merge it before the ASKEM evaluation ends in 2 weeks.

SamWitty commented 7 months ago

@eb8680 , now that the ASKEM evaluation is over can I review this?

eb8680 commented 7 months ago

Sure, go for it.