Skip to content

Update parameters and dependencies of all learners #46

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

Merged
merged 22 commits into from
Nov 12, 2019
Merged

Conversation

be-marc
Copy link
Member

@be-marc be-marc commented Oct 18, 2019

Closes #9
Closes #27

  • Glmnet
    Updated to glmnet 3.0

  • KKNN
    Not sure if y-kernel should be implemented

  • LDA

  • LogReg

  • NaiveBayes

  • QDA

  • Ranger

  • SVM

  • Xgboost
    Parameters updated in Update xgboost learner to latest CRAN version #31

  • KM

  • LM

  • CV parameter for internal cross-validation are not included

  • Should we include na.action if the options are fail or omit records with missing values? This would add the missing property to the learner but only if na.action is set to omit. Some of the learners in the list support this.

  • We could add parameters like offset, contrast and singular.ok to LogReg and LM? But they were not included in mlr. Do we want this?

@pat-s
Copy link
Member

pat-s commented Oct 21, 2019

Thanks for cleaning up!

@codecov-io
Copy link

codecov-io commented Oct 25, 2019

Codecov Report

Merging #46 into master will decrease coverage by 0.07%.
The diff coverage is 97.46%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
- Coverage   97.61%   97.53%   -0.08%     
==========================================
  Files          18       18              
  Lines         630      691      +61     
==========================================
+ Hits          615      674      +59     
- Misses         15       17       +2
Impacted Files Coverage Δ
R/LearnerClassifXgboost.R 97.8% <ø> (ø) ⬆️
R/LearnerClassifQDA.R 91.66% <100%> (+1.19%) ⬆️
R/LearnerClassifRanger.R 96.07% <100%> (+0.24%) ⬆️
R/LearnerRegrSVM.R 100% <100%> (ø) ⬆️
R/LearnerClassifSVM.R 100% <100%> (ø) ⬆️
R/LearnerRegrGlmnet.R 96.72% <100%> (+0.35%) ⬆️
R/LearnerClassifNaiveBayes.R 100% <100%> (ø) ⬆️
R/LearnerClassifGlmnet.R 96.92% <100%> (+0.14%) ⬆️
R/LearnerClassifLDA.R 92.3% <100%> (+1.39%) ⬆️
R/LearnerRegrRanger.R 96.15% <100%> (+0.5%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 543b37b...5a3ade6. Read the comment docs.

Copy link
Member

@pat-s pat-s left a comment

Choose a reason for hiding this comment

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

We could add parameters like offset, contrast and singular.ok to LogReg and LM? But they were not included in mlr. Do we want this?

If there is no reason mentioned why they were omitted: Then yes, add them. Most likely they were added recently and just not updated yet in mlr2.


Could you please add the changes to NEWS.md? Not every single param ofc, just mentioning all updated learners and maybe version info for bigger changes like glmnet and xgboost.

Co-Authored-By: Patrick Schratz <patrick.schratz@gmail.com>
be-marc and others added 2 commits November 12, 2019 10:54
Co-Authored-By: Patrick Schratz <patrick.schratz@gmail.com>
Co-Authored-By: Patrick Schratz <patrick.schratz@gmail.com>
@pat-s pat-s changed the title Check parameters and dependencies Update parameters and dependencies of all learners Nov 12, 2019
@pat-s pat-s merged commit e5fd62a into master Nov 12, 2019
@pat-s pat-s deleted the check_parameters branch November 12, 2019 10:00
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.

add dependencies on parameters for all learners Check learner parameters
3 participants