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

CI Metrics #522

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open

CI Metrics #522

wants to merge 13 commits into from

Conversation

sofiene-boutaj
Copy link

Add metrics (R2 score, MCC, MK, lr-, lr+) and non parametric (bootstrap) confidence intervals for classification, regression and reconstruction.

@pep8speaks
Copy link

pep8speaks commented Jan 29, 2024

Hello @sofiene26000! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-02-16 14:03:50 UTC

Copy link
Member

@NicolasGensollen NicolasGensollen left a comment

Choose a reason for hiding this comment

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

Thanks @sofiene26000 !
I had a quick look and made some suggestions to make the code a bit easier to read.

clinicadl/utils/maps_manager/maps_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/maps_manager/maps_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/maps_manager/maps_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/maps_manager/maps_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/maps_manager/maps_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/metric_module.py Outdated Show resolved Hide resolved
else:
results = dict()

return results

@staticmethod
def mae_fn(y, y_pred):
def mae_fn(y, y_pred, *args):
Copy link
Member

Choose a reason for hiding this comment

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

Not sure whether ClinicaDL is using type hints extensively or not... Maybe @camillebrianceau knows better ?

clinicadl/utils/task_manager/classification.py Outdated Show resolved Hide resolved
clinicadl/utils/task_manager/task_manager.py Outdated Show resolved Hide resolved
clinicadl/utils/task_manager/reconstruction.py Outdated Show resolved Hide resolved
@sofiene-boutaj
Copy link
Author

Thanks @sofiene26000 ! I had a quick look and made some suggestions to make the code a bit easier to read.

Thank you for your feedback! I will try to include it in my next commit.

@camillebrianceau camillebrianceau self-assigned this Feb 14, 2024
@camillebrianceau camillebrianceau added this to the v1.6.0 milestone Feb 14, 2024
@camillebrianceau camillebrianceau modified the milestones: v1.6.0, v1.6.1 Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants