-
Notifications
You must be signed in to change notification settings - Fork 942
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
Vectorize operations for propensity score matching #1179
Conversation
Thanks for starting this, @rahulbshrestha . Let us know once the PR is ready for review. |
I ran some tests to check if the values of att and atc are the same before and after changes made in this PR:
and the results when running on some test data:
Both lists, treated outcomes and control outcomes are the same before and after the changes I made. The ATT and ATC seems to be off by a couple digits after averaging (check last 3 digits in the example above), which is probably a rounding error. Is this a problem @amit-sharma? |
Added todo comment Signed-off-by: Rahul Shrestha <rahulshrestha0101@gmail.com> formatting fix Signed-off-by: Rahul Shrestha <rahulshrestha0101@gmail.com> bug fix with string name Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
Signed-off-by: rahulbshrestha <rahulshrestha0101@gmail.com>
Hey @amit-sharma! I think this PR is ready to be merged :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @rahulbshrestha . The changes look good!
@all-contributors please add @rahulbshrestha for code. |
I've put up a pull request to add @rahulbshrestha! 🎉 |
This PR addresses this issue by introducing vectorized operations instead of the existing for-loops. This should speed up operations for large datasets.
This PR is a work in progress, and the remaining tasks include: