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

not so small-ish fixes #79

Merged
merged 21 commits into from
Apr 7, 2021
Merged

not so small-ish fixes #79

merged 21 commits into from
Apr 7, 2021

Conversation

jeremyrcoyle
Copy link
Collaborator

@jeremyrcoyle jeremyrcoyle commented Feb 24, 2021

Some small-ish fixes throughout:

  • Correct the bounding code in predict to work for a matrix of predictions (from multiple lambda)
  • Change the default lambda.min.ratio to 1e-4 (The previous default overpenalized)
  • Porting over the changes to the HAL formula functionality originally contributed in Redo formula version of fit_hal #80
  • Removal of outdated cpp files that contained documentation but no actual code at all
  • Changes to Rcpp documentation to suppress Roxygen errors (NOTE: the line before // [[Rcpp::export]] cannot be a comment (i.e., //), but must be valid Rcpp-Roxygen (e.g., //')
  • Reduce redundant Rcpp functions, eliminating get_pnz and get_xscale, which were exact copies of calc_pnz and calc_xscale

R/make_basis.R Outdated Show resolved Hide resolved
@nhejazi nhejazi changed the title small fixes small-ish fixes Mar 28, 2021
R/hal.R Outdated Show resolved Hide resolved
@nhejazi nhejazi changed the title small-ish fixes not so small-ish fixes Mar 29, 2021
@jeremyrcoyle
Copy link
Collaborator Author

@nhejazi, we fixed this up this morning. Should be good to go, pending a look from you. Thanks for your help with it!

@nhejazi
Copy link
Member

nhejazi commented Apr 4, 2021

Great, I just added some more documentation/style edits. Assuming my most recent commits don't fail the CI build checks, then this LGTM for merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants