Skip to content

Conversation

gisilvs
Copy link

@gisilvs gisilvs commented Jul 22, 2025

What does this PR do?

Fixes #3110

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--3186.org.readthedocs.build/en/3186/

@SkafteNicki SkafteNicki self-assigned this Aug 4, 2025
@SkafteNicki SkafteNicki added enhancement New feature or request and removed waiting on author labels Aug 4, 2025
@SkafteNicki SkafteNicki added this to the v1.9.0 milestone Aug 4, 2025
@SkafteNicki
Copy link
Collaborator

I manage to refactor the new dinov2 feature extractor into the existing fid implementation keeping backwards compatibility

Copy link

codecov bot commented Aug 4, 2025

Codecov Report

❌ Patch coverage is 19.29825% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 37%. Comparing base (10f48cc) to head (3450cef).
⚠️ Report is 3 commits behind head on master.

❌ Your project check has failed because the head coverage (37%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (10f48cc) and HEAD (3450cef). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (10f48cc) HEAD (3450cef)
gpu 1 0
unittest 1 0
Additional details and impacted files
@@            Coverage Diff            @@
##           master   #3186      +/-   ##
=========================================
- Coverage      99%     37%     -62%     
=========================================
  Files          15     349     +334     
  Lines         195   19938   +19743     
=========================================
+ Hits          194    7376    +7182     
- Misses          1   12562   +12561     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

seems it is not compatible with python 3.9

@mergify mergify bot added the has conflicts label Aug 6, 2025
@mergify mergify bot removed the has conflicts label Aug 8, 2025
@Borda Borda requested a review from Copilot September 19, 2025 19:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for DinoV2 feature extractors to the FrechetInceptionDistance metric, expanding beyond the original InceptionV3-only implementation. The changes introduce string-based feature specification and deprecate integer inputs while maintaining backward compatibility.

  • Adds NoTrainDinoV2 class for DinoV2 feature extraction
  • Introduces string-based feature specification (e.g., "inception-2048", "dino-768")
  • Updates API to use feature_extractor instead of inception attribute names

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/torchmetrics/image/fid.py Core implementation adding DinoV2 support, feature mapping, and API updates
tests/unittests/image/test_fid.py Comprehensive test updates for new feature extractors and string-based configurations
CHANGELOG.md Documents the new DinoV2 feature extractor support

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

self.inception = NoTrainInceptionV3(
name="inception-v3-compat",
self.feature_extractor = NoTrainInceptionV3(
name=f"inception-{feature}",
Copy link
Preview

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The name parameter should be 'inception-v3-compat' to maintain compatibility with the original implementation, not f'inception-{feature}'. This change could break existing functionality that depends on the specific model name.

Suggested change
name=f"inception-{feature}",
name="inception-v3-compat",

Copilot uses AI. Check for mistakes.

],
)
def test_compare_fid(tmpdir, equal_size, feature_config):
"""Check that the whole pipeline gives the same result as torch-fidelity."""
Copy link
Preview

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

Typo in comment: 'whole' should be 'hole' to maintain consistency with the original comment.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: Image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frechet Inception Distance with Dinov2 features
3 participants