-
Notifications
You must be signed in to change notification settings - Fork 614
Fix kappa #1047
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
Fix kappa #1047
Conversation
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
@facaiy @seanpmorgan @WindQAQ This is the best I could think of to keep things as simple and clean as possible. But if you feel we need to change something, I am open to suggestions. |
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.
Mostly LGTM, one edge case I would like to see tested.
As a side note thank you very much for the comprehensive RFC. It made following the issue substantially easier.
Thanks @seanpmorgan for the feedback. I have renamed that variable and added more tests to cover all the cases. |
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.
Very nice fix thanks again! LGTM
We'll add this to 0.8.2 patch release |
Cool. Thanks for the valuable feedback |
* fix kappa * add more tests and rename regression variable * add cross_entropy test for binary class model (cherry picked from commit 83df531)
* Fix kappa (#1047) * fix kappa * add more tests and rename regression variable * add cross_entropy test for binary class model (cherry picked from commit 83df531) * Typo in type #1067 (#1069) (cherry picked from commit 99352d0) * Bump to 0.8.2 * Add CI to release branches Co-authored-by: Aakash Kumar Nain <[email protected]> Co-authored-by: failure-to-thrive <[email protected]>
* fix kappa * add more tests and rename regression variable * add cross_entropy test for binary class model
This PR fixes the problems mentioned in #906
Highlights: