ecpolley / SuperLearner

Current version of the SuperLearner R package
272 stars 72 forks source link

Allow passing of single 'split' index to SampleSplitSuperLearner() #103

Closed saraemoore closed 6 years ago

saraemoore commented 6 years ago

I'm doing something akin to leave-one-out CV, but with observation-specific weights and columns of X. I'm currently using this fork's version of SampleSplitSuperLearner() to accomplish this by specifying a single row of X as the validation sample. All other solutions that I've come up with thus far either require unnecessary additional sample splitting (for example, passing in the validation observation via newX) or unnecessary duplication of code (for example, writing a custom GridLearner(), like a SuperLearner() that doesn't cross-validate).

Previously, SampleSplitSuperLearner() needed at least two observations to be in the validation set if specific row numbers of X were provided. I modified SampleSplitSuperLearner() to accept only one specific observation as the validation sample, while still maintaining the ability to pass in a proportion of the sample to use as the training set instead of row numbers. Let me know if you foresee any issues with this modification.

ck37 commented 6 years ago

I haven't had a chance to check out yet, but would it be possible to create a very simple test for this function?

saraemoore commented 6 years ago

Good idea. I'll do that now and update the PR.

saraemoore commented 6 years ago

I looked through the failed checks briefly, and I don't believe any of these are attributable to my changes. The build failures seem to be mostly (entirely?) due to problems with dependencies like BioC, quadprog, nlopt, etc.

ecpolley commented 6 years ago

Thanks, I don't foresee a problem with this change, it just wasn't a use case when SampleSplit was created. One minor note, can you leave the version number in the Description, but add a comment in the inst/NEWS file with the description of the change?

saraemoore commented 6 years ago

Cool.

Changes now made. I updated the date in both DESCRIPTION and NEWS.