Skip to content

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.

@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
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.

2 participants