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

new metric SCC for Images #800

Conversation

nishant42491
Copy link

@nishant42491 nishant42491 commented Jan 26, 2022

What does this PR do?

Adds a new metric Spatial Correlation Coefficient for Images

part of #799

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)-Yes
  • Did you read the contributor guideline, Pull Request section?-Yes
  • Did you make sure to update the docs?-No
  • Did you write any new necessary tests?-No

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 🙃

@justusschock justusschock marked this pull request as draft January 26, 2022 10:59
@nishant42491 nishant42491 changed the title Demo Pull Request Draft Pull Request for Issue 799 Jan 26, 2022
@Borda Borda changed the title Draft Pull Request for Issue 799 new metric SCC for Images Jan 26, 2022
@Borda Borda mentioned this pull request Jan 26, 2022
@codecov
Copy link

codecov bot commented Jan 26, 2022

Codecov Report

Merging #800 (9d6449b) into master (640c4d1) will increase coverage by 0%.
The diff coverage is 95%.

@@          Coverage Diff          @@
##           master   #800   +/-   ##
=====================================
  Coverage      71%    71%           
=====================================
  Files         179    181    +2     
  Lines        7664   7744   +80     
=====================================
+ Hits         5429   5505   +76     
- Misses       2235   2239    +4     

@SkafteNicki SkafteNicki added this to the v0.8 milestone Jan 26, 2022
@SkafteNicki
Copy link
Member

Hi @nishant42491, thanks for wanting to contribute.
However, I can only see that you have created a new file. Is that correct?

@nishant42491
Copy link
Author

Hi @nishant42491, thanks for wanting to contribute. However, I can only see that you have created a new file. Is that correct?

yes, that's correct. sorry for the delay as I was trying to understand how to calculate the SCC coefficient as there are not many resources available online to understand the implementation of Laplacian filters. I am working with my college professors to understand more about the SCC metric how ever due to my college exams my progress has been slow. however I will try to speed things up again really sorry for the delay.

@stancld
Copy link
Contributor

stancld commented Mar 25, 2022

Hi @nishant42491, do you have any updates here? :]

@nishant42491
Copy link
Author

Hi @nishant42491, do you have any updates here? :]

yep, I have taken references from andrewkhalels sewar repo containing information about the SCC metric and have created a scc.py file containing its implementation however I am not 100 % sure that it's correct as I still don't understand the SCC metric completely. Ill be more than happy to change anything that needs changing. I would love any suggestions on how to proceed further. Thank you a lot for your patience with me I really appreciate it :].

@Borda Borda modified the milestones: v0.8, v0.9 Apr 5, 2022
@SkafteNicki
Copy link
Member

Hi @nishant42491,
I tried to take a stab at finishing this PR. I written tests and added some docs, but the tests fails with assertion when I try to compare against the implementation from sewar. Could you maybe help with getting this PR finished, by providing at least an example how to get this implementation to match the one from sewar.

@nishant42491
Copy link
Author

Hi @nishant42491, I tried to take a stab at finishing this PR. I written tests and added some docs, but the tests fails with assertion when I try to compare against the implementation from sewar. Could you maybe help with getting this PR finished, by providing at least an example how to get this implementation to match the one from sewar.

Hi @nishant42491, I tried to take a stab at finishing this PR. I written tests and added some docs, but the tests fails with assertion when I try to compare against the implementation from sewar. Could you maybe help with getting this PR finished, by providing at least an example how to get this implementation to match the one from sewar.

I think the difference between the two metrics arises because the package in swear has implemented a high pass filter whilst I have not however the difference did not seem significant whilst i was testing the metric

@nishant42491
Copy link
Author

@SkafteNicki should I try implementing a high pass filter for my metric too to try and make it pass the tests you have written?

@SkafteNicki
Copy link
Member

@nishant42491 yes, we kind of need the two metrics to produce the same output, so we make sure that this implementation is doing the right thing.
Maybe the reason the tests are failing is that the input is random? did you test with some structured input?

@nishant42491
Copy link
Author

image

image

image

image

![puppy](https://user-images.githubusercontent.com/79035403/165307101-b9628ea4-bf33-4c70-95c2-f16c83995358.jpeg) ![mud_turtle](https://user-images.githubusercontent.com/79035403/165307128-2da3fc20-14fe-4dc1-9f13-719b9ffd98bb.jpg)

@SkafteNicki I have received outputs 0.079 and 0.080 for 2 images for swear's metric and my metric respectively

@nishant42491
Copy link
Author

will try and implement the high pass filter which should solve the problem. @SkafteNicki Thnx for the help on the tests i really appreciate it :]

@SkafteNicki
Copy link
Member

@nishant42491 is it correct that current differences in implementation is:

  • The input to sewar should be target, preds whereas your implementation takes in oppesit order preds, target (which is the same as the rest of out codebase)
  • The input to sewar is expected to be single images of shape [H, W, C] whereas your implementation takes in [B, C, H, W] (so the channel dimension should be moved when we compare)

Just trying to figure out why I cannot get the numbers to match (I know the high pass filter is missing, but it should still be fairly close as to my understanding).

@nishant42491
Copy link
Author

@nishant42491 is it correct that current differences in implementation is:

  • The input to sewar should be target, preds whereas your implementation takes in oppesit order preds, target (which is the same as the rest of out codebase)
  • The input to sewar is expected to be single images of shape [H, W, C] whereas your implementation takes in [B, C, H, W] (so the channel dimension should be moved when we compare)

Just trying to figure out why I cannot get the numbers to match (I know the high pass filter is missing, but it should still be fairly close as to my understanding).

yep, those are the differences between the sewar implementation and the metric's implementation. for the inputs, I have tested
In both metrics the difference is slight generally up to the 3rd decimal place.

@nishant42491
Copy link
Author

@nishant42491 is it correct that current differences in implementation is:

  • The input to sewar should be target, preds whereas your implementation takes in oppesit order preds, target (which is the same as the rest of out codebase)
  • The input to sewar is expected to be single images of shape [H, W, C] whereas your implementation takes in [B, C, H, W] (so the channel dimension should be moved when we compare)

Just trying to figure out why I cannot get the numbers to match (I know the high pass filter is missing, but it should still be fairly close as to my understanding).

yep, those are the differences between the sewar implementation and the metric's implementation. for the inputs, I have tested In both metrics the difference is slight generally up to the 3rd decimal place.

one more thing is that my default kernel size is 9, whilst sewras kernel size is 8 so you have to set the sewar metrics kernel size to 9 explicitly to get similar results

@SkafteNicki
Copy link
Member

@nishant42491 tried changing the implementation based on what you have told me but cannot get it to match. Below is shown the wrapped reference implementation from sewar that should do everything correctly:

  • summarizes a batch of input
  • flips the input
  • permute the dimensions
  • changes the window size to 9
from sewar.full_ref import scc

def _reference_scc(preds, target, reduction):
    val = 0.0
    for p, t in zip(preds, target):
        val += scc(t.permute(1, 2, 0).numpy(), p.permute(1, 2, 0).numpy(), ws=9)
    val = val if reduction == "sum" else val / preds.shape[0]
    return val

from torchmetrics.functional import spatial_correlation_coefficient

import torch
_ = torch.manual_seed(42)

BATCH_SIZE = 10
CHANNELS = 3
SIZE = 100
preds = torch.randint(0, 255, (BATCH_SIZE, CHANNELS, SIZE, SIZE)).float()
target = torch.randint(0, 255, (BATCH_SIZE, CHANNELS, SIZE, SIZE)).float()

print(spatial_correlation_coefficient(preds, target, reduction='sum').item())
print(_reference_scc(preds, target, reduction='sum'))

can you find what I do wrong?

@nishant42491
Copy link
Author

@nishant42491 tried changing the implementation based on what you have told me but cannot get it to match. Below is shown the wrapped reference implementation from sewar that should do everything correctly:

  • summarizes a batch of input
  • flips the input
  • permute the dimensions
  • changes the window size to 9
from sewar.full_ref import scc

def _reference_scc(preds, target, reduction):
    val = 0.0
    for p, t in zip(preds, target):
        val += scc(t.permute(1, 2, 0).numpy(), p.permute(1, 2, 0).numpy(), ws=9)
    val = val if reduction == "sum" else val / preds.shape[0]
    return val

from torchmetrics.functional import spatial_correlation_coefficient

import torch
_ = torch.manual_seed(42)

BATCH_SIZE = 10
CHANNELS = 3
SIZE = 100
preds = torch.randint(0, 255, (BATCH_SIZE, CHANNELS, SIZE, SIZE)).float()
target = torch.randint(0, 255, (BATCH_SIZE, CHANNELS, SIZE, SIZE)).float()

print(spatial_correlation_coefficient(preds, target, reduction='sum').item())
print(_reference_scc(preds, target, reduction='sum'))

can you find what I do wrong?

Your Implementation seems correct. I'm not quite sure why the outputs differ.

@Borda
Copy link
Member

Borda commented May 5, 2022

@nishant42491 @SkafteNicki how is it going here? 🐰
we are considering a new feature release next week, do you think we can finish this addition by then so it will be included? 🙏

@SkafteNicki SkafteNicki modified the milestones: v0.9, v0.10 May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants