-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
[ENH] Optimize Unsuprf #114
Conversation
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #114 +/- ##
==========================================
+ Coverage 86.52% 86.73% +0.21%
==========================================
Files 26 26
Lines 2063 2111 +48
==========================================
+ Hits 1785 1831 +46
- Misses 278 280 +2
☔ View full report in Codecov by Sentry. |
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
@PSSF23 can you add the support to honest tree and forest for partial fit to fix the CI here?
Feel free to commit directly to the branch once you have something working locally? Afterwards, I can merge this PR and the forests/trees in scikit-tree should have partial_fit enabled. |
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.
@adam2392 I have optimized the honest fitting as we currently have classes
as a variable. Right now only the samples used for tree structures are included in the fitting, instead of the current sample_weight=0
method.
|
There are some errors in the implementation. Also do you mind adding the functionality to the forest as well? I'm not as sure what needs to get added to partial_fit. As long as it passes the |
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.
I think all CIs work now.
Cool thanks @PSSF23! |
Towards: #113
Changes proposed in this pull request:
Before submitting
section of the
CONTRIBUTING
docs.Writing docstrings section of the
CONTRIBUTING
docs.After submitting