[SYSTEMDS-3681] Add stepGLM builtin function. - #2523
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2523 +/- ##
============================================
+ Coverage 71.38% 71.66% +0.27%
- Complexity 48753 49336 +583
============================================
Files 1571 1580 +9
Lines 188912 190517 +1605
Branches 37067 37364 +297
============================================
+ Hits 134857 136533 +1676
+ Misses 43593 43464 -129
- Partials 10462 10520 +58 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
christinadionysio
left a comment
christinadionysio
left a comment
There was a problem hiding this comment.
Thank you for you PR, @Bruno-Ignaz!
I have a few minor things that should be improved before we can push your code into the upstream. It would be appreciated if you could have a look.
| if (nrow(B) != 3 | sum(beta_est != 0 & beta_true == 0) > 0 | sum(beta_est == 0 & beta_true != 0) > 0) { | ||
| stop("Test failed: Inexact feature support recovery."); | ||
| } | ||
| print("passed test 1") |
There was a problem hiding this comment.
It would be great if we could create two tests that are called from the BuiltinSTEPGlmTest.java so that we know exactly which test is failing without the need for print statements.
|
|
||
|
|
||
|
|
||
| #stop("!!!Sucess!!!") # uncomment for letting the test fail |
There was a problem hiding this comment.
Please remove this line.
| if (max(abs(beta_est - beta_true)) > epsilon) { | ||
| stop("Test failed: Parameter estimates exceed tolerance bound epsilon = " + epsilon + "."); | ||
| } | ||
| print("passed test 2") |
There was a problem hiding this comment.
Same as above, create specific test case that is called from the java test suite, and remove the print statement.
| all_ones = matrix (1, rows = num_records, cols = 1); | ||
| [AIC_best, ignore_beta2, ignore_S2] = internal_glm(X=all_ones, Y=Y, intercept_status=0, num_features_orig=num_features, Selected=columns_fixed_ordered, link=link, disp=disp, tol=tol, moi=moi, mii=mii); | ||
| } | ||
| #print ("Best AIC without any features: " + AIC_best); |
There was a problem hiding this comment.
Please remove this line
| } | ||
|
|
||
| if (column_best == 0) { | ||
| #print ("AIC of an empty model is " + AIC_best + " and adding no feature achieves more than " + (thr * 100) + "% decrease in AIC!"); |
There was a problem hiding this comment.
Please remove this line.
| } | ||
| }; | ||
|
|
||
| # print ("Best AIC " + AIC_best + " achieved with feature: " + column_best); |
There was a problem hiding this comment.
Please remove this line.
| [AIC_best, ignore_beta4, ignore_S4] = internal_glm(X=X_global, Y=Y, intercept_status=0, num_features_orig=num_features, Selected=columns_fixed_ordered, link=link, disp=disp, tol=tol, moi=moi, mii=mii); | ||
| } else { | ||
| # compute AIC of an empty model with only intercept (all Ys are constant) | ||
| ###all_ones = matrix (1, rows = num_records, cols = 1); |
There was a problem hiding this comment.
This should be removed as well.
|
|
||
| # cbind best found features (i.e., columns) to X_global | ||
| if (as.scalar(columns_fixed[1,column_best]) == 0) { # new best feature found | ||
| #print ("Best AIC " + AIC_best + " achieved with feature: " + column_best); |
| } | ||
|
|
||
| # run GLM with selected set of features | ||
| print ("Running GLM with selected features..."); |
There was a problem hiding this comment.
Please do not add print statements.
|
|
||
| [AIC, B, S] = stepGLM::m_stepGLM(X=X, Y=Y, link=2, yneg=0.0, icpt=0, tol=1e-6, disp=0.0, moi=200, mii=0, thr=0.01); | ||
|
|
||
| print("\n\n\n\n\n\n\nTest Results:"); |
There was a problem hiding this comment.
Please remove the print statements.
github-project-automation
Bot
moved this from In Progress
to In Review
in SystemDS PR Queue
2 participants
Hi,
This is a homework pull-requests for the course AMLs at TUB.
This is also my first pull-request :)
I refactored
./scripts/algorithm/StepGLM.dmlinto a importable function.glm_fitwith adapter that uses the builtin solverm_gmlfrom./scripts/builtin/gml.dmlThe probit inverse link function is now approximated (AIC calculation):
Additionally:
./scripts/algorithms/TestBuiltinStepGLM.dmlperforms two checks for function output.Sources
[1]: Takeshi, Qualitative Response Models: A Survey (1981)