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

Change semantic ordering of kernel_size parameter in SSIM #474

Merged
merged 6 commits into from
Aug 25, 2021

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Aug 23, 2021

Before submitting

  • Was this discussed/approved 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?

What does this PR do?

Fixes #414
Changes the semantic ordering of kernel_size parameter of SSIM metric such that it matches the standard [N, C, H, W] image format. Note this does not change anything for the end user, essentially just a change of variable naming to match standard.
Also adds a bit more testing for the metric.

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 🙃

@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #474 (3b0f104) into master (548a597) will decrease coverage by 21%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #474    +/-   ##
======================================
- Coverage      96%    75%   -21%     
======================================
  Files         130    130            
  Lines        4301   4301            
======================================
- Hits         4125   3232   -893     
- Misses        176   1069   +893     

@SkafteNicki SkafteNicki added enhancement New feature or request test / CI testing or CI labels Aug 23, 2021
@mergify mergify bot added the ready label Aug 24, 2021
@SkafteNicki SkafteNicki enabled auto-merge (squash) August 24, 2021 07:49
@mergify mergify bot removed the has conflicts label Aug 24, 2021
@Borda
Copy link
Member

Borda commented Aug 25, 2021

@justusschock mind have look?

@SkafteNicki SkafteNicki merged commit 94a158c into master Aug 25, 2021
@SkafteNicki SkafteNicki deleted the ssim_kernel_order branch August 25, 2021 12:41
Borda pushed a commit that referenced this pull request Aug 27, 2021
* implem

* changelog

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
(cherry picked from commit 94a158c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready test / CI testing or CI topic: Image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ambiguity in semantic ordering of kernel_size and padding computation in SSIM
3 participants