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

PUBDEV-7973 glm beta constraints not working #5842

Merged
merged 6 commits into from
Nov 11, 2021

Conversation

wendycwong
Copy link
Contributor

@wendycwong wendycwong commented Oct 19, 2021

This PR fixes the problems in JIRA: https://h2oai.atlassian.net/browse/PUBDEV-7973.

This is a two part JIRA. One is to fix the problem with beta constraints not working and the other one is to add additional ways to specify beta constraints per Megan Kurka suggestion

Done:

  1. Fixed bugs associated with beta constraints. I added additional application of beta constraints to the end of LS if it exists, else, it will be applied after the new beta is calculated;
  2. I extended beta constraints to be applicable to IRLSM as well as COORDINATE_DESCENT and LBFGS.
  3. Added tests to make sure that the coefficients are constraints are limited as desired.
  4. I enabled that python users can now specify beta_constraints using a python dict per Megan Kurka suggestions.
  5. Added tests to make sure the new way of specifying beta constraints using python dict works.

@wendycwong wendycwong force-pushed the PUBDEV-7973-glm-beta-constraints-problems branch 3 times, most recently from 75b9ef3 to a157cc2 Compare October 28, 2021 20:03
@wendycwong wendycwong force-pushed the PUBDEV-7973-glm-beta-constraints-problems branch from 5e476ea to 5971e81 Compare October 29, 2021 23:28
@wendycwong
Copy link
Contributor Author

Finished the performance test and it does not look like the fixes here affected the training time:

image

@wendycwong wendycwong force-pushed the PUBDEV-7973-glm-beta-constraints-problems branch from bc2d2b3 to 45b5aea Compare November 4, 2021 16:37
@michalkurka
Copy link
Contributor

@wendycwong can we also add a self-check that will test that the coefficients after training are within the bounds == the algorithm will do the check itself and will never return unexpected results

@wendycwong
Copy link
Contributor Author

@michalk: Good idea.

Copy link
Contributor

@michalkurka michalkurka left a comment

Choose a reason for hiding this comment

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

Thank you Wendy - looks great, well tested

@wendycwong wendycwong force-pushed the PUBDEV-7973-glm-beta-constraints-problems branch from e32234d to 747c6b5 Compare November 8, 2021 22:52
Add Michalk test
Fix bug to change beta for numerical colummns
Move beta constraints after application of LS
added beta constraints tests
Fixed bug on when to add beta constraints
Add coefficient magnitude check to tests to make sure limits are respected.
Allow user to specify beta_constraints as a python dict.
Fixed bug when _non_negative=True
Fixed Junit smoke test failure
revert COD beta constraints but add extra after LS
added final self-check to make sure coefficients are within beta constraints
Copy xgboost and make the check for beta constraints defeatable for debugging purposes
Rename monotonicity check to beta constraints check
@wendycwong wendycwong force-pushed the PUBDEV-7973-glm-beta-constraints-problems branch from 747c6b5 to 9ce31e6 Compare November 8, 2021 23:05
Copy link
Contributor

@tomasfryda tomasfryda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you @wendycwong !

@wendycwong wendycwong merged commit d481285 into rel-zizler Nov 11, 2021
@wendycwong wendycwong deleted the PUBDEV-7973-glm-beta-constraints-problems branch November 11, 2021 17:16
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.

3 participants