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

Skip unrolling follow up #3260

Merged
merged 15 commits into from
Jul 16, 2024
Merged

Conversation

simeetnayan81
Copy link
Contributor

@simeetnayan81 simeetnayan81 commented Jul 13, 2024

Description: Follow up PR for adding argument skip_unrolling in Metric Class. #3258
Following classes need to be updated:

  • Accuracy
  • AveragePrecision
  • CohenKappa
  • ConfusionMatrix
  • CosineSimilarity
  • Entropy (Only docstring update is required)
  • EpochMetric
  • Frequency (We may skip this)
  • JSDivergence (Only docstring update is required)
  • KSDivergence (Only docstring update is required)
  • MaximumMeanDiscrepancy
  • MeanAbsoluteError (Only docstring update is required)
  • MeanPairwiseDistance
  • MeanSquaredError (Only docstring update is required)
  • MultiLabelConfusionMatrix
  • MutualInformation (Only docstring update is required)
  • PrecisionRecallCurve
  • Precision
  • PSNR
  • Recall
  • ROC_AUC
  • RootMeanSquaredError (Only docstring update is required)
  • RunningAverage
  • SSIM
  • TopKCategoricalAccuracy

Modification might also be required to related classes.

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: metrics Metrics module label Jul 13, 2024
@simeetnayan81
Copy link
Contributor Author

This is just an initial PR. I am currently working on the remaining classes.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR with all those meticulous updates!
I left few comments to improve it.

ignite/metrics/average_precision.py Show resolved Hide resolved
ignite/metrics/mean_pairwise_distance.py Outdated Show resolved Hide resolved
tests/ignite/metrics/test_average_precision.py Outdated Show resolved Hide resolved
tests/ignite/metrics/test_cohen_kappa.py Outdated Show resolved Hide resolved
tests/ignite/metrics/test_epoch_metric.py Show resolved Hide resolved
@simeetnayan81
Copy link
Contributor Author

I don't think these classes require skip_unrolling feature. Lmk.
Frequency, GpuInfo, MetricsLamda

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jul 16, 2024

Yes, we can skip GpuInfo and MetricsLambda, but we still need to add it to Frequency for consistency.

@simeetnayan81
Copy link
Contributor Author

Have updated the Frequency class as well. Args were missing in docstring, added them.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for the PR, @simeetnayan81 !
Good to go once the CI is green (as we ignore failing TPU test)

@vfdev-5 vfdev-5 merged commit 6b6b169 into pytorch:master Jul 16, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: metrics Metrics module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants