search

alan-turing-institute / cloudcasting

Cloud modelling (Manchester Prize)
MIT License
2 stars 0 forks source link

Make SSIM work with NaN tests #52

Closed dfulu closed 1 month ago

dfulu commented 1 month ago

Note: edited since first posted

By changing some of the parameters used in the structural_similarity() function from skimage, we can get the SSIM calculation to work with the examples in our tests which contain NaNs in the target image.

In working on this, we discovered that the old configuration of structural_similarity() only fails if there is a NaN in the top left (i.e. at y[..., 0, 0]) but doesn't fail if there are NaNs elsewhere. So we had only caught an edge case with our tests.

However, I believe that this new configuration of structural_similarity() which passes with our current tests is still better because it is more robust and more referenceable. The new configuration of structural_similarity() matches that used in the original paper . We use the hint in the structural_similarity() docstring to match to the original paper.

This solves #11

phinate commented 1 month ago

Just going through this to make sure I understand it. I thought I'd look at the SSIM between a random image from the leading channel and a blurry version to just see what happens. Top is the normal output of SSIM, bottom cuts out the border.

Here, the effect of NaNs appearing doesn't materialize. Do I need to be looking at all channels to see this working?

image
IFenton commented 1 month ago

LGTM

phinate commented 1 month ago

So for documentation purposes, we had a discussion on this. Our tests were failing because of the very particular choice of putting a NaN in the top-left corner of our image:

https://github.com/alan-turing-institute/cloudcasting/blob/324be76f66608d39936665d5f7f217ce5b8ea313/tests/conftest.py#L38

There is some interaction here between this and setting gaussian_weights = False. We avoid this here by consistently using gaussian_weights = True everywhere, and fix the hyperparameters to match those in the original structural similarity paper. We didn't track down a concrete explanation of the all-NaN behaviour, but are satisfied that we now have a numerically stable output in the case of NaNs in the input!