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

Add plot functionality to AUROC and ROC metrics #1490

Merged
merged 30 commits into from
Feb 24, 2023
Merged

Add plot functionality to AUROC and ROC metrics #1490

merged 30 commits into from
Feb 24, 2023

Conversation

alexkrz
Copy link
Contributor

@alexkrz alexkrz commented Feb 7, 2023

What does this PR do?

Fixes part of #1406

Currently, plot functionality is added to modular metrics BinaryAUROC, MulticlassAUROC, MultilabelAUROC and BinaryROC.

Before submitting
  • Was this discussed/agreed via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?
PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@mergify mergify bot added the has conflicts label Feb 7, 2023
@alexkrz
Copy link
Contributor Author

alexkrz commented Feb 8, 2023

Not sure why the docs test is failing. Docs could be build successfully on my local machine.
Also one question: I have adopted the examples from the MulticlassAccuracy metric and I am using random numbers for preds and target. Is there a fixed seed such that the examples generated in the docs turn out the same on every build?

requirements/image.txt Outdated Show resolved Hide resolved
src/torchmetrics/classification/auroc.py Outdated Show resolved Hide resolved
src/torchmetrics/classification/auroc.py Outdated Show resolved Hide resolved
src/torchmetrics/classification/auroc.py Outdated Show resolved Hide resolved
src/torchmetrics/classification/auroc.py Outdated Show resolved Hide resolved
src/torchmetrics/classification/roc.py Outdated Show resolved Hide resolved
src/torchmetrics/classification/roc.py Outdated Show resolved Hide resolved
Copy link
Member

@SkafteNicki SkafteNicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already looking good :)
Could you please also add test to this file:
https://github.com/Lightning-AI/metrics/blob/master/tests/unittests/utilities/test_plot.py

.gitignore Outdated Show resolved Hide resolved
requirements/image.txt Outdated Show resolved Hide resolved
src/torchmetrics/classification/auroc.py Show resolved Hide resolved
src/torchmetrics/classification/roc.py Outdated Show resolved Hide resolved
@alexkrz
Copy link
Contributor Author

alexkrz commented Feb 12, 2023

Already looking good :) Could you please also add test to this file: https://github.com/Lightning-AI/metrics/blob/master/tests/unittests/utilities/test_plot.py

This is actually my first time contributing to an open source project and I am new to working with pytest. I have now included tests for BinaryAUROC and MulticlassAUROC plots. I would be glad if you could have a look on it.
I can also try to write a test for the plot_binary_roc_curve() function from src/utilities/plot.py

@mergify mergify bot removed the has conflicts label Feb 20, 2023
@codecov
Copy link

codecov bot commented Feb 20, 2023

Codecov Report

Merging #1490 (fc4b1c9) into master (82d5271) will decrease coverage by 0%.
The diff coverage is 56%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1490   +/-   ##
======================================
- Coverage      88%     88%   -0%     
======================================
  Files         216     216           
  Lines       11344   11396   +52     
======================================
+ Hits         9969    9997   +28     
- Misses       1375    1399   +24     

@Borda Borda requested review from Borda and SkafteNicki February 21, 2023 11:44
@Borda Borda added the enhancement New feature or request label Feb 21, 2023
@mergify mergify bot added the ready label Feb 21, 2023
@mergify mergify bot removed the has conflicts label Feb 24, 2023
@Borda Borda merged commit 7fa72c9 into Lightning-AI:master Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants