-
Notifications
You must be signed in to change notification settings - Fork 935
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
Remove use of CausalModel from notebooks and test files #1220
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
@bloebp I did it for one file, could you please let me know if this is fine? The only thing is when I tried to compare the variables for identify_effect for before and after, it seems to be different? e.g if I do
The output is False, any idea why? I can dig into this a bit more too. |
thanks for starting this @rahulbshrestha |
@amit-sharma I investigated this a bit more and found the difference between both:
Is this an intended design choice or a bug? |
Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
Hey, I think @amit-sharma is better in answering this. Generally, I would expect that we obtain 1:1 the same. If not, there might be an issue we should investigate. |
@rahulbshrestha yeah you are right! I checked the two implementations of So we'll need to implement a custom |
This PR is stale because it has been open for 60 days with no activity. |
This PR was closed because it has been inactive for 7 days since being marked as stale. |
@amit-sharma any chance to take a look? |
yeah, this PR needs a bit of more work. We'll need to implement |
This PR is stale because it has been open for 60 days with no activity. |
This PR addresses this issue. Since
CausalModel
is deprecated, it should be removed from tutorials and test cases.