kboyd / Roc

Everything ROC and Precision-Recall curves.
BSD 2-Clause "Simplified" License
23 stars 7 forks source link

Issue15 fix version 2 #18

Closed kboyd closed 10 years ago

kboyd commented 10 years ago

Better bug fix for integer overflow of #15, should be used instead of #17. No changes to internal Curve representation, only cast to double before performing totalPositives * totalNegatives operation in rocArea and mannWhitneyU.

I'm casting to double instead of long since the ultimate result of the expression is always a double. If you think a more accurate computation will occur by casting to long, performing the multiplication, and only then casting to double I can do it but that involves more casts and seems confusing.

daslu commented 10 years ago

Thank you for the fix!

Yes, casting just before multiplication seems to be the appropriate change, sorry if I caused the confusion.

I read into the code, this looks fine. I will test it with large examples in the next following days.

afbarnard commented 10 years ago

Summarizing our conversation today, these are the things I would like to see added/fixed.

At some point we should have more scale tests, but that doesn't need to happen now.

kboyd commented 10 years ago

Updated branch with the following fixes.

afbarnard commented 10 years ago

Ok, @kboyd, this all looks good. Per your request, I'll do the honors.