Hi @pranjaldatta is the image gradient metric not defined as the difference between the image gradient of a ground true image and a predicted image?

Hi @pranjaldatta is the image gradient metric not defined as the difference between the image gradient of a ground true image and a predicted image?

Hey @SkafteNicki, image-gradient functions by definition return dy/dx of a single image. (following from sklearn/TF implementations). If you are referring to a class-based (like here), that can be done, but I assumed since image-gradients are usually added as a base to loss functions like depth loss, etc., it would make more sense to have a depth-loss metric implemented separately (class and functional) and functional implementation of image-gradients that is used by these metrics and can be used otherwise also.

I wanted to add depth-loss too, but I read in the guidelines that a single PR should only cater to a single issue, hence I thought adding depth-loss would be violating those. But I'll be happy to make any changes if you could guide me a bit regarding the correct approach.

@pranjaldatta okay I better understand the purpose of this PR now.
I agree that keeping PRs as focused as possible is better as it is easier to review.
Please rename the PR like "Image gradient metric 1/n" where n should be the number of PRs you intent to do on this topic.

Please rename the PR like "Image gradient metric 1/n" where n should be the number of PRs you intent to do on this topic.

@SkafteNicki I have changed the name of the PR. I intend on opening another Issue regarding depth-loss that builds on this. Hope this is appropriate.

Codecov Report

Merging #5056 (579414f) into release/1.2-dev (5f94900) will increase coverage by 0%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##           release/1.2-dev   #5056   +/-   ##
===============================================
  Coverage               93%     93%           
===============================================
  Files                  147     148    +1     
  Lines                10419   10441   +22     
===============================================
+ Hits                  9652    9674   +22     
  Misses                 767     767           

I understand that a lot of tests are failing and I see that almost all of them have something to do with "PyTorch compiled without the CUDA version". Is this because I am using the CPU-only version of PyTorch installed via pip? How can I solve this?

Considering I am a bit of a beginner when it comes to OSS contributions, I would be incredibly grateful for a bit of help here.

CC @tchaton @SkafteNicki

There is a single test failing: related to docker. I cannot understand whether the error is something to do with the code or it's some transient error. Any help is highly appreciated.

Hi @pranjaldatta, sorry for not getting back to you before now.
Failing tests are unrelated to your PR.

Could you please add a reference to your metric in https://github.com/PyTorchLightning/pytorch-lightning/blob/master/docs/source/metrics.rst
Then I think this PR is ready to be merged.

Hey @SkafteNicki, no worries.
Regarding the addition to the doc, just to confirm, is it appropriate for me to add the reference to image gradient here, https://github.com/PyTorchLightning/pytorch-lightning/blob/master/docs/source/metrics.rst#functional-metrics-regression ? i.e. under the heading "Functional Metrics (Regression)".

Hey @SkafteNicki, no worries.
Regarding the addition to the doc, just to confirm, is it appropriate for me to add the reference to image gradient here, https://github.com/PyTorchLightning/pytorch-lightning/blob/master/docs/source/metrics.rst#functional-metrics-regression ? i.e. under the heading "Functional Metrics (Regression)".

Yes that seems to be the appropriate place. Please, also add a note in the changelog file under the "added" section.

Hey @SkafteNicki, I have updated the docs and changelog as asked!

@SkafteNicki added a commit making all the recommended changes.

@pranjaldatta I still see many comments and sure if they are implemented or not, mind chek them and if they are done, pls mark them as resolved...

@Borda Sorry about the clutter, all the conversations had been solved with the last commit. Have resolved all of them now!

@Borda I have made the changes via the last commit!