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 backprop in LPIPS #2326

Merged
merged 14 commits into from
Feb 12, 2024
Merged

Fix backprop in LPIPS #2326

merged 14 commits into from
Feb 12, 2024

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Jan 29, 2024

What does this PR do?

Fixes #2315
The LPIPS metric should allow for a loss to be back propagated through it as a loss function, however the parameters of the metric are not intended by default to also update. This PR should fix this.

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

@SkafteNicki SkafteNicki added the bug / fix Something isn't working label Jan 29, 2024
@SkafteNicki SkafteNicki added this to the v1.3.x milestone Jan 29, 2024
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Merging #2326 (a7c233c) into master (b6f6e07) will decrease coverage by 31%.
Report is 4 commits behind head on master.
The diff coverage is 21%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #2326     +/-   ##
========================================
- Coverage      69%     38%    -31%     
========================================
  Files         305     305             
  Lines       17164   17180     +16     
========================================
- Hits        11836    6578   -5258     
- Misses       5328   10602   +5274     

@mergify mergify bot added ready and removed ready labels Feb 6, 2024
@mergify mergify bot added ready and removed ready labels Feb 6, 2024
@Borda Borda enabled auto-merge (squash) February 7, 2024 16:24
@mergify mergify bot added the ready label Feb 7, 2024
@mergify mergify bot added the has conflicts label Feb 8, 2024
@mergify mergify bot removed the has conflicts label Feb 9, 2024
@mergify mergify bot removed the has conflicts label Feb 12, 2024
@Borda Borda disabled auto-merge February 12, 2024 12:39
@Borda Borda merged commit 18b181d into master Feb 12, 2024
51 of 58 checks passed
@Borda Borda deleted the bugfix/lpips_backprop branch February 12, 2024 12:40
Borda pushed a commit that referenced this pull request Feb 12, 2024
* fixes + tests
* fix doctests

(cherry picked from commit 18b181d)
Borda pushed a commit that referenced this pull request Feb 12, 2024
* fixes + tests
* fix doctests

(cherry picked from commit 18b181d)
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 topic: Image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LPIPS gives negative value
2 participants