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

Fix MetricCollection with repeated compute calls #2211

Merged
merged 9 commits into from
Nov 28, 2023

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Nov 14, 2023

What does this PR do?

Fixes #2206

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 🙃


📚 Documentation preview 📚: https://torchmetrics--2211.org.readthedocs.build/en/2211/

@SkafteNicki SkafteNicki added the bug / fix Something isn't working label Nov 14, 2023
@SkafteNicki SkafteNicki added this to the v1.2.x milestone Nov 14, 2023
@SkafteNicki SkafteNicki self-assigned this Nov 14, 2023
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Merging #2211 (d9e3e59) into master (9652899) will increase coverage by 0%.
Report is 1 commits behind head on master.
The diff coverage is 100%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #2211   +/-   ##
======================================
  Coverage      87%     87%           
======================================
  Files         293     293           
  Lines       16425   16426    +1     
======================================
+ Hits        14279   14280    +1     
  Misses       2146    2146           

CHANGELOG.md Show resolved Hide resolved
@Borda Borda enabled auto-merge (squash) November 14, 2023 17:25
@mergify mergify bot removed the has conflicts label Nov 25, 2023
@Borda Borda disabled auto-merge November 28, 2023 16:04
@Borda Borda merged commit 58ffb01 into master Nov 28, 2023
47 of 56 checks passed
@Borda Borda deleted the collection_repeat_compute branch November 28, 2023 16:06
Borda pushed a commit that referenced this pull request Nov 30, 2023
Co-authored-by: Jirka Borovec <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 58ffb01)
Borda pushed a commit that referenced this pull request Dec 1, 2023
Co-authored-by: Jirka Borovec <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 58ffb01)
relativityhd added a commit to relativityhd/torchmetrics that referenced this pull request May 28, 2024
Also update tests to correctly test for Lightning-AI#2211.
Also add return deepcopy values of compute
SkafteNicki added a commit that referenced this pull request Jun 1, 2024
* Fix incorrect caching of MetricCollection

Also update tests to correctly test for #2211.
Also add return deepcopy values of compute

* Update Changelog

* fix issues with deepcopy by using .clone

---------

Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jirka Borovec <[email protected]>
Borda pushed a commit that referenced this pull request Aug 2, 2024
* Fix incorrect caching of MetricCollection

Also update tests to correctly test for #2211.
Also add return deepcopy values of compute

* Update Changelog

* fix issues with deepcopy by using .clone

---------

Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jirka Borovec <[email protected]>
(cherry picked from commit 6a23b38)
Borda pushed a commit that referenced this pull request Aug 2, 2024
* Fix incorrect caching of MetricCollection

Also update tests to correctly test for #2211.
Also add return deepcopy values of compute

* Update Changelog

* fix issues with deepcopy by using .clone

---------

Co-authored-by: Nicki Skafte Detlefsen <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Jirka Borovec <[email protected]>
(cherry picked from commit 6a23b38)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MetricCollection just recomputes the first metric after first .compute call if compute_groups is enabled
3 participants