Warnings fixed for test_ssim.py

[bibhabasumohapatra]

Fixes #2358

Description: tests/ignite/metrics/test_ssim.py::test_ssim
/home/runner/work/ignite/ignite/tests/ignite/metrics/test_ssim.py:79: FutureWarning: multichannel is a deprecated argument name for structural_similarity. It will be removed in version 1.0.Please use channel_axis instead.

Solved : I have changed the multichannel = True to channel_axis = -1, as multichannel is a deprecated argument.

Check list:

• fixed the warning of deprecated argument.

[sdesrozis]

@bibhabasumohapatra Thank you ! LGTM !

[sdesrozis]

Regarding to the doc, channel_axis=-1 is similar to multichannel=False.

I don’t understand the reason why you replaced channel_axis=3. Could you give some feedback on that please ?

[bibhabasumohapatra]

scikit-image/scikit-image#4294
in this discussion they agreed upon,

multichannel=True becomes channel_axis=-1.
multichannel=False becomes channel_axis=None.

and in the test_ssim.py

 y_pred = torch.rand(8, 3, 224, 224, device=device)
 _test_ssim(
        y_pred, y, data_range=1.0, kernel_size=7, sigma=1.5, gaussian=False, use_sample_covariance=True, device=device
    )

and inside def _test_ssim():
we are doing,
skimg_pred = y_pred.permute(0, 2, 3, 1).cpu().numpy()

So any ways I do it , whether I do channel_axis = -1 or channel_axis = 3 it would work,
but I think its more professional to write channel_axis = -1 and according to skimage discussion on this topic that I linked,
multichannel was discontinued for the reason that scikit-image/scikit-image#4294 (comment)
its my first try to contribute, please correct me if I read or interpreted anything wrong,
I think channel_axis = -1 is what we should go with,
thanks @sdesrozis

[sdesrozis]

Ok, according to the discussion, I would prefer having channel_axis=3 which seems to be a correct usage in our case.

what do you think ?

[bibhabasumohapatra]

as we used multichannel = True its the hard coded version of channel_axis = -1 so that is the reason I changed , anyways its the same , and fixes the warnings and as the members of skiimage treated multichannel I think we should treat it the same way so if in future someone refers to it, and does not check permute(0, 2, 3, 1) and(8, 3, 224, 224)would assume last to represent channel i.e axis = -1

(B,H,W,C) is the form we are using after permute, we say last is 'C' or 3 (4th) of 0,1,2,3 indices is 'C'.
again quoting the general take:

multichannel=True becomes channel_axis=-1.
multichannel=False becomes channel_axis=None.

like if we were using (B,C,H,W) then we would have done channel_axis = 1 ,
so my take is if I am writing the code , then I would have kept channel_axis = 3, as I am changing the depracated multichannel , so I would prefer channel_axis = -1 to be in the continuation wrt past.

[sdesrozis]

IMO the value -1 could be deprecated in the next version since it's like a patch. It's safer to have the correct usage of channel_axis especially if you have checked the results are the same.

As you mentioned, you would be able to avoid reordering using correctly this parameter and consider the shape (B, C, W, H) with channel_axis=1. Could you try this please ? Thanks again for your help !

[bibhabasumohapatra]

great! I have done that ... lets deprecate the old permute and use channel_axis.

[sdesrozis]

A remark to address before approval.

[bibhabasumohapatra]

as per @sdesrozis suggestion to use channel_axis = 1 to avoid reordering in permute(0,2,3,1) that is making B C H W format to B H W C.
So I removed all the permute(0,2,3,1) and instead made channel_axis = 1

[vfdev-5]

@bibhabasumohapatra thanks for the PR. Issues with mypy are unrelated. I'll fix them in #2362. Once it is merged please sync your branch with the master.

[sdesrozis]

LGTM. @bibhabasumohapatra Thank you !

[vfdev-5]

@bibhabasumohapatra @sdesrozis failure on python 3.6 seems related

[bibhabasumohapatra]

@bibhabasumohapatra @sdesrozis failure on python 3.6 seems related

but 3.7 and 3.8 are not showing failure if I am not wrong.
I am new to GitHub and Open source especially going through logs,
in the logs I found that it shows:

im1 = array([[[[1.40380979e-01, 7.68278778e-01, 5.81218898e-01, ...,
          3.63162220e-01, 4.09203708e-01, 3.27855110e-0...1.56306088e-01, 2.11606205e-01, ...,
          2.36398578e-02, 2.62628496e-01, 1.32986903e-01]]]],
      dtype=float32)
im2 = array([[[[9.12476331e-02, 4.99381185e-01, 3.77792269e-01, ...,
          2.36055434e-01, 2.65982389e-01, 2.13105813e-0...1.01598956e-01, 1.37544021e-01, ...,
          1.53659070e-02, 1.70708522e-01, 8.64414871e-02]]]],
      dtype=float32)
win_size = 11, gradient = False, data_range = 1.0, multichannel = False
gaussian_weights = True, full = False, kwargs = {'channel_axis': 1}

 if np.any((np.asarray(im1.shape) - win_size) < 0):
            raise ValueError(
>               "win_size exceeds image extent.  If the input is a multichannel "
                "(color) image, set multichannel=True.")
E           ValueError: win_size exceeds image extent.  If the input is a multichannel (color) image, set multichannel=True.

.. versionchanged:: 0.16
This function was renamed from skimage.measure.compare_ssim to
skimage.metrics.structural_similarity. is 0.16 old version?
I think like it expects Multichannel! But we know its deprecated. in the same logs its mentioned multichannel parameter but no channel_axis parameter
Python 3.6 support is going to be removed, right?

[sdesrozis]

Yes python 3.6 implies an old version of skimage. We have planned to remove python 3.6, so I would agree to do this before merge this PR.

@vfdev-5 What do you think ?

[vfdev-5]

Removing python 3.6 from CI does not solve a potential issue, right ?

OK, I see the issue is related to installed scikit-image version installed for python 3.6. Scikit-image officially dropped py36: https://pypi.org/project/scikit-image/#files

Before we drop python 3.6 on our CI, here we can skip this test and merge it.

[vfdev-5]

LGTM, thanks @bibhabasumohapatra !