Undesired behaviour of warp_to_mask in MaskedImage

[jalabort]

The method warp_to_mask in MaskedImage always warps the mask on the particular MaskedImage from which the method is executed.

While this makes a lot of sense in general, it might cause problems when using AAMs. In particular, for some edge cases the warped image and the reference frame end up having different number of pixels which causes the AAM fitting to crash.

This can be easily solved by bringing back the old kwarg warp_mask but because warp_to_mask is a very general method I though I would open this issue for further discussion.

[jabooth]

The method warp_to_mask in MaskedImage always warps the mask on the particular MaskedImage from which the method is executed.

Exactly correct

This can be easily solved by bringing back the old kwarg warp_mask but because warp_to_mask is a very general method I though I would open this issue for further discussion.

I would strongly argue against this. The above definition you gave is simple to understand - warp_to_mask on a MaskedImage unambiguously always warps the mask. If you don't want that behaviour, then don't use a MaskedImage (easy with the new API):

warped_img = masked_img.warp_to_mask(template, warp_mask=False)  # old API
warped_img = masked_img.as_unmasked().warp_to_mask(template)  # new API

#369 was a huge change and I am not surprised to find we are now finding the edge cases. I think the right response is to be more disciplined, and stop using masked images were they really don't make sense.

For instance I don't think it ever makes sense to build an AAM from masked images. Instances that the AAM returns should be masked images, because that is the way in Menpo that we say these pixel values should not be interpreted as meaning anything. The images used in construction require no such definition.

The edge case errors we are hitting now are equivalent to the statement:

I want to sample a pixel from a region of the image that is explicitly marked as not being usable.

In the context of AAM construction, we know the response we want to give is:

Stop being pedantic! Don't worry, that pixel is fine to be used, can you please just warp it?.

In which case - why mask it in the first place?

@jalabort can you confirm this error is arising due to us building AAMs from MaskedImage instances, or is there another area where we need to be stricter?

[jalabort]

This is arising when trying to fit MaskedImages.

You have a good point there, maybe we do not want to fit MaskedImages...

[jabooth]

Yes, the same argument holds for fitting. The idea was now that menpo.io imports Image and not MaskedImage by default people would be dissuaded from fitting and building with MaskedImage. Did you manually turn them into masked images before fitting, or were they being returned to you as masked from some API in Menpo without you knowingly doing it? If so I'd say that's a bug.

[jalabort]

I was turning them to MaskedImages for visualization and then trying to fit them.

OK, if you guys all knew about this and are happy with the current behaviour then it is also fine by me. I just found it strange that it crashed, hence the issue.

[jabooth]

Ok. I'll leave this open as a reminder that we should ensure in all notebooks and Menpo itself we don't promote using masked images in the fitting framework. Once we are happy everything is converted we can close this.

[jabooth]

Decision from meeting - raise a Warning on training and fitting if you use MaskedImages.

[jabooth]

@jalabort did you look into this? I think we just decided to raise a warning if using masked images in fitting. This will now be on menpofit, so once it's done over there we can close this issue.

[patricksnape]

This has changed now with the introduction of #563. Now, an explicit exception is raised (OutOfMaskSampleError) when you attempt to warp a masked image outside of it's mask. However, we still need to make sure that as_unmasked is called in Menpofit for AAMs.

For this reason, I'm going to close this and open a Menpofit issue about it.