-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Bugfix/hardwired sigmoid #3850
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
Bugfix/hardwired sigmoid #3850
Conversation
…f Microsoft.ML.StandardTrainers
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.
A few minor comments but it's almost good to go!
Thanks a lot for contributing this fix! Would you consider making a direct test between ML.NET and LightGBM using non-default sigmoid? We have a similar test here. |
Thanks for your feedback @wschin ! I added a positive and negative test case for the case you mentioned. |
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.
Overall LGTM. Just some minor comments. :)
Co-Authored-By: Wei-Sheng Chin <[email protected]>
Co-Authored-By: Wei-Sheng Chin <[email protected]>
Co-Authored-By: Wei-Sheng Chin <[email protected]>
This reverts commit 6722dbf. # Conflicts: # test/Microsoft.ML.Tests/TrainerEstimators/TreeEstimators.cs
Fixes #1422
Fixes the Hardcoded Sigmoid value from -.5 to the value specified during training.
Let Microsoft.ML.Tests see internals of Microsoft.ML.Standard trainers to access trained model parameters for testing verification.