Fix overflow errors

[fhausmann]

Reference Issues/PRs

#624

What does this implement/fix? Explain your changes.

This PR converts the input of several metrics from int to float. This has the advantage that the value range is larger and no overflow occurs. Thus, the correct values are also reported for large integers.

Any other comments?

Thank you for pyCM. It is really great!

[sepandhaghighi]

@fhausmann

I would like to express my gratitude for your interest in the PyCM project.

Please review the following points:

1. Review this link, which suggests that the pull request should be opened from a branch under the dev branch (containing the latest version) to the original dev branch.
2. Can you clarify the reasoning behind applying this casting to only specific functions, and why only those functions encountered overflow issues?
3. Instead of creating a new file, please integrate the overflow test into the overall_test.py file at the end.

[fhausmann]

@sepandhaghighi Thanks for your feedback. I, unfortunately, didn't see the contributing guidelines. I'm sorry about that.

1. The merge request is now against the dev branch.

2. I didn't want to change too much. For these functions, I got warnings regarding overflows, so I primarily focused on those. The reason behind this is probably that these functions involve multiplications of the large values, e.g., ACC and MCC have the same inputs, but MCC involves multiplication, and ACC just involves additions. Thus, MCC reaches overflows more easily. There might be cases where other metrics also reach overflow issues. While thinking about it, maybe converting values earlier in the process to floats might make more sense, e.g., in class_statistics(). However, then probably FP, TP, FN, ... would be reported as floats instead of ints. Would that be a problem?

3. Tests are moved.

[sepandhaghighi]

@sepandhaghighi Thanks for your feedback. I, unfortunately, didn't see the contributing guidelines. I'm sorry about that.

1. The merge request is now against the dev branch.
2. I didn't want to change too much. For these functions, I got warnings regarding overflows, so I primarily focused on those. The reason behind this is probably that these functions involve multiplications of the large values, e.g., ACC and MCC have the same inputs, but MCC involves multiplication, and ACC just involves additions. Thus, MCC reaches overflows more easily. There might be cases where other metrics also reach overflow issues. While thinking about it, maybe converting values earlier in the process to floats might make more sense, e.g., in class_statistics(). However, then probably FP, TP, FN, ... would be reported as floats instead of ints. Would that be a problem?
3. Tests are moved.

Well done!
I will send the PR to the review queue.
About the second point: Yes, it's not a good practice to report TP/TN/FP/FN as floats.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.47%. Comparing base (757d018) to head (ac2a2e6).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #625      +/-   ##
==========================================
+ Coverage   99.46%   99.47%   +0.01%     
==========================================
  Files          15       15              
  Lines        3323     3347      +24     
  Branches      446      446              
==========================================
+ Hits         3305     3329      +24     
  Misses         12       12              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fhausmann]

I checked and found also a case where the values are so large that even integer additions are not possible anymore.
This is early on recognized when computing TP, FP, FN, .... For this case, I added a decorator, which raises an actual error instead of only a runtime warning. I think with these two changes (decorator and fixed metrics), no false values in case of an overflow are reported.

[sepandhaghighi]

@fhausmann
Thank you for your efforts. It appears that there is an issue in Python 3.7 and Python 3.8 tests on Windows. It seems it's because of the numpy version; I will work on it and then start the review.

[sepandhaghighi]

@fhausmann, I want to thank you once again for your contribution! 💯
I made some edits to your pull request and removed the decorator. While your overall approach to the decorator was correct, it caused issues in some versions of Python and wasn't stable. We will now begin the review process.

[sepandhaghighi]

@alirezazolanvari @sadrasabouri

Please review this pull request (PR). Keep in mind that the CI failure is unrelated to the changes made in this PR. It is due to the deprecation of the numpy.trapz function, which falls outside the scope of this pull request. We should address that issue in a separate PR. Thank you!

[sadrasabouri]

Thanks for opening this PR @fhausmann, I think it's good to add this precomputation float conversion to accracy calcilcation function as well.

[sadrasabouri]

Looks good to me now.

[sepandhaghighi]

🔥