Skip to content

Conversation

@szmazurek
Copy link
Collaborator

Fixes #741

Proposed Changes

  • modify the requirements in setup.py to use torchmetrics 1.1.2
  • modify the metrics library for compatibility with the new torchmetrics API
  • modified the tests to be compatible with new functionalities (mostly related to the class constructor arguments)

Checklist

  • I have read the CONTRIBUTING guide.
  • My PR is based from the current GaNDLF master .
  • Non-breaking change (does not break existing functionality): provide as many details as possible for any breaking change.
  • Function/class source code documentation added/updated.
  • Code has been blacked for style consistency.
  • If applicable, version information has been updated in GANDLF/version.py.
  • If adding a git submodule, add to list of exceptions for black styling in pyproject.toml file.
  • Usage documentation has been updated, if appropriate.
  • Tests added or modified to cover the changes; if coverage is reduced, please give explanation.
  • If customized dependency installation is required (i.e., a separate pip install step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].

szmazurek and others added 4 commits November 19, 2023 08:31
- added requirements in setup.py
- modified the metrics constructors in the GANDLF.metrics.generic and .classification
files to be compatibile with updated torchmetrics API
- modified .yaml file to cover new fields compatibile with new torchmetrics API
- defined utility functions for easier construction of metrics from new torchmetrics API
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2023

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@codecov
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3f1b8a0) 94.69% compared to head (c0d251f) 94.67%.

Files Patch % Lines
testing/test_full.py 99.16% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #742      +/-   ##
==========================================
- Coverage   94.69%   94.67%   -0.03%     
==========================================
  Files         117      117              
  Lines        8224     8242      +18     
==========================================
+ Hits         7788     7803      +15     
- Misses        436      439       +3     
Flag Coverage Δ
unittests 94.67% <99.34%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@sarthakpati sarthakpati changed the title Upgrade torchmetrics to 1.1.2 Upgrade base torchmetrics version Nov 20, 2023
@sarthakpati sarthakpati self-requested a review November 20, 2023 12:58
Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

Minor changes requested. Additionally, Codacy errors [ref] need to be fixed as well:

image

szmazurek and others added 9 commits November 20, 2023 17:46
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Import Hotfix
sarthakpati and others added 5 commits November 21, 2023 10:03
- removed redundant if-else statement
- fixed bug with incorrect reading from the average_types_keys dictionary
- moved utility functions into utils.generic.py
- simplified the creation of generic metrics
- modified the usage docs to link to the current torchmetrics version docs
@szmazurek szmazurek closed this Nov 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2023
@szmazurek szmazurek reopened this Nov 22, 2023
@mlcommons mlcommons unlocked this conversation Nov 22, 2023
Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

Requesting minor changes - check linting

@sarthakpati sarthakpati changed the title Upgrade base torchmetrics version Upgrade base torchmetrics version Nov 22, 2023
szmazurek and others added 9 commits November 22, 2023 15:09
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Co-authored-by: Sarthak Pati <patis@iu.edu>
Copy link
Collaborator

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

LGTM

@sarthakpati sarthakpati merged commit 10821bd into mlcommons:master Nov 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade torchmetrics from 0.8.1 to 1.1.2

2 participants