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

Added external predictions for PLR model #204

Merged

Conversation

JanTeichertKluge
Copy link
Member

PLR extended by the option to use external predictions

Description

The PLR model was extended by the possibility to calculate causal effects from externally generated predictions. This has been implemented along the lines of the IRM model.
As with the IRM model, the _nuisance_est method has been adapted so that it offers the possibility of passing external predictions in the fit method of the parent class.
In addition, the unit tests for comparing the values calculated manually and in the package have been extended. These now also compare the results between predictions generated with nuisance learner inside the package or predictions generated outside.

.gitignore Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this ignore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested other things besides the Unittest/PyTest, but I did not want to track the script for this, so it should be ignored. I can also move this script to another folder so that it is untracked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that might be preferable. Or you can remove it after the merge.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change in the .gitignore file should no longer exist. Thank you for pointing this out.

Copy link
Member

@SvenKlaassen SvenKlaassen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, but why do we need the gitignore?

removing testfile
@JanTeichertKluge JanTeichertKluge merged commit 3c12101 into DoubleML:s-include-pred Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants