-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
surv.aorsf
: mtry_ratio
can result in mtry = 1
, which causes an error
#259
Comments
So I suggest to:
@bcjaeger Do you think that is a good idea? |
I'm a little worried about tuning though. |
Hi! Thanks for noticing this,
I do, both 1. and 2. make sense. I could remove this error from
This is a good point. For |
I don't know if/how we can, since Allowing |
That definitely makes sense. I will check out If |
I think a warning would be better and would solve the issue. |
It looks like allowing As far as changes in mlr3 go, I would still be in support of @sebffischer's solution if you would like to avoid seeing a warning message about 1 predictor being used in |
Unfortunately I don't really know the algorithm. Is it possible that there are situations where one predictor might be better than 2? In that case I would even argue that the warning should be submitted and users should know what they are doing when they are setting parameters. I would argue the warning belongs into the documentation and not the code. But I think it is your call @bcjaeger |
Decision trees can be axis based or oblique. Axis based trees use one predictor to partition data into two new branches of the tree, while oblique trees use a linear combination of predictors. A linear combination of one predictor is the same thing as the original predictor as far as a decision tree split is concerned, so an oblique tree with one predictor is the same thing as an axis based one. I'm okay with putting the warning into documentation. I would guess that using |
Thanks a lot! :) |
Hello! Just wanted to let you know library(aorsf)
orsf(pbc_orsf, time + status ~ . -id, mtry = 1)
#> ---------- Oblique random survival forest
#>
#> Linear combinations: Accelerated
#> N observations: 276
#> N events: 111
#> N trees: 500
#> N predictors total: 17
#> N predictors per node: 1
#> Average leaves per tree: 20
#> Min observations in leaf: 5
#> Min events in leaf: 1
#> OOB stat value: 0.84
#> OOB stat type: Harrell's C-statistic
#> Variable importance: anova
#>
#> ----------------------------------------- Created on 2022-12-14 with reprex v2.0.2 This change will be in version 0.0.5 of |
The learner failed when setting mtry to 1. This was addressed in the new aorsf release. #259
Description
mtry_ratio
can take valid values that, depending on the task, result in invalid values formtry
.Maybe less a bug than likely undesired behavior where I'm not sure if this can/should be addressed in the learner or left as an exercise to the user, so to speak.
When
mtry_ratio
has an implicit lower bound not equal to 0, this can result in unintended behavior in benchmark scenarios where feature counts may vary widely across tasks, butmtry_ratio
is tuned within [0, 1].I'm not sure if there's anything that could be done learner-wise without causing other issues tho :/
That being said, I guess at least the
mtry
lower bound should be increased, as it's currently set to 1:mlr3extralearners/R/learner_aorsf_surv_aorsf.R
Line 40 in 5c7780e
Reproducible example
Created on 2022-12-07 with reprex v2.0.2
Session info
The text was updated successfully, but these errors were encountered: