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

Issue #625: solve aggregation issue for test.join with probs #1275

Merged
merged 2 commits into from
Oct 13, 2016

Conversation

giuseppec
Copy link
Contributor

Try to solve #625

@larskotthoff
Copy link
Member

Could you add the example from the issue report as a test please?

@giuseppec
Copy link
Contributor Author

giuseppec commented Oct 7, 2016

Done.

Note: For repeated-CV test.join currently computes the measure for each repeated cv separately and then averages this through the mean. For example, a 3 times repeated 10-fold CV creates cross-validated predictions for each repetition, e.g. y1, y2, and y3. In this case, currently test.join computes the measure for y1 and the real y, y2 and the real y, y3 and the real y and finally aggregates the three values using the mean. (why aggregating here? especially why using the mean?).
An alternative to this (without the issue of aggregating) would be to average the cross-validated predictions accross all reps, i.e., use the mean ymean = (y1+y2+y3)/3 and compute the measure using ymean and the real y.

@larskotthoff
Copy link
Member

Hmm, sounds to me like this should be configurable by the user (in particular how the aggregation is done). This is probably a separate PR though?

@giuseppec
Copy link
Contributor Author

Should we first merge this PR that fixes a bug? And then open up a new issue for that as this is rather an enhancement and no bug.

@larskotthoff
Copy link
Member

Sounds good. I'll merge this now.

@larskotthoff larskotthoff merged commit 501e60b into master Oct 13, 2016
@larskotthoff larskotthoff deleted the fix_625_aggregate_prob_for_test.join branch October 13, 2016 07:46
@giuseppec giuseppec mentioned this pull request Oct 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants