zachmayer / caretEnsemble

caret models all the way down :turtle:
http://zachmayer.github.io/caretEnsemble/
Other
226 stars 75 forks source link

Opt tests #84

Closed jknowles closed 9 years ago

jknowles commented 9 years ago

Adds tests for optAUC functions and optRMSE functions. Changes safeOptAUC and greedOptAUC to issue a warning and a message respectively in case of optimization underperforming a component model. In the case of safeOptAUC, the best model is weighted 1 and returned, in greedOptAUC the best optimization is returned with a message.

Basic tests for greedOptRMSE and ability to issue a message in degenerate cases. However, no tests for degenerate cases are included because I couldn't generate any data to do this.

Closes #75.

jknowles commented 9 years ago

Errr. Don't merge this in. I am realizing we have a bit of an issue with how to handle missing predictions all the way down the stack of functions.

I'm currently diagnosing this and seeing what solution we might want to come up with for the short term and the long term.

zachmayer commented 9 years ago

Sounds good!

zachmayer commented 9 years ago

@jknowles Once you've fixed the missing data issue, we can re-visit this.

Here's another idea: once of the caretEnsemble graphs should be a plot of the model's error vs. iteration number. If the error is increasing, the user can tell somethings gone wrong with the optimization.

jknowles commented 9 years ago

The build is passing, but NA resolution is still not transparent to the user. I am going to add a few more tests and push them to this branch. The optimizers are performing correctly, but they are all using na.rm=TRUE with no user option to do anything else and no warning about missing values.

zachmayer commented 9 years ago

Just let me know when you think this is ready to merge!

jknowles commented 9 years ago

I think if this build passes the Travis CI check, it should be ready to merge.

zachmayer commented 9 years ago

Looks pretty good to me. I have a few comments prior to merging (I just got back from vacation, so sorry it took a while for me to get around to this!)

jknowles commented 9 years ago

OK -- I cleaned up the style and undid my change to optRMSE. We should be set.

Note that this branch makes optRMSE behave like optAUC in that model weights are optimized based on cases which data is available across all models in the library. Previously, AUC did this by default through the colAUC function, but now the RMSE optimizer behavior does the same thing. No warning or message is given to the user, but I noted this in the documentation for caretEnsemble and the optimizer functions.

This closes a couple remaining issues for v. 1.0 I think too.

zachmayer commented 9 years ago

Sounds good. I think making them consistent makes sense, and we can decide how to better deal with NAs later.

jknowles commented 9 years ago

Great. Thanks for reviewing this -- no problem with the vacation! We are very close to a 1.0 release now!

zachmayer commented 9 years ago

One more issue to fix!!