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

Consider unify the sanitize inputs for every estimator #93

Open
CrystalXuR opened this issue Apr 11, 2022 · 4 comments
Open

Consider unify the sanitize inputs for every estimator #93

CrystalXuR opened this issue Apr 11, 2022 · 4 comments

Comments

@CrystalXuR
Copy link
Contributor

No description provided.

@CrystalXuR
Copy link
Contributor Author

@erikcs Do you think these checks on the inputs at https://github.com/som-shahlab/survmetabench/blob/master/r-package/survlearners/R/surv_rl_grf.R#L86 are necessary? I adopted this from R-learner, but I don't have such checks in my other estimators, so I am thinking of either remove it for all or add it for all

@erikcs
Copy link
Collaborator

erikcs commented Apr 11, 2022

Yes input checks are necessary, there are some common ones like: 1) make sure all dims align in X,Y,W,D 2) they are numeric (e.g X can not be a string/factor for glmnet/grf...) 3) predict(object, newdata) newdata needs same dim as X.train that can be a separate function in utils

there are also estimator specific checks e.g. if all D=0 or all D=1 then your code will most likely break in places where you do a subset like D==0 and continue using the result, or subset W==1 if there are no treated (or too few treated if you make folds on that subset)

@erikcs
Copy link
Collaborator

erikcs commented Apr 11, 2022

we can do a meet first after the 1-6 in #22 is done

@CrystalXuR
Copy link
Contributor Author

Sounds good, thanks

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

No branches or pull requests

2 participants