ImagePixelExtractingEstimator should not require the ImageResizingEstimator in the pipeline

[sfilipi]

Currently in ML.Net, to be able to use an ImagePixelExtractingTransformer/ImagePixelExtractingEstimator, you need to have an ImageResizingEstimator in the pipeline before the ImagePixelExtracting, otherwise the pipeline fails at run-time with a schema mismatch error.

"'Schema mismatch for input column 'ImageObject': expected known-size image, got unknown-size image'".

This happen because the ImagePixelExtracting operating on ImageTypes needs the Width and the Height properties, to extract the pixels.

The Estimator that creates the ImageType object, is the ImageLoadingEstimator, but the Width and Height properties of the estimator don't get filled until they get passed to ImageResizing.

Proposed fix:

• The ImageLoadingEstimator should allow the user to specify the height and width of the images, if the user has images of fixed with and height, rather than fail at "ExtractPixels" if resize is not present.
• if we make a bool about imagesHaveSameDimentions, and that is set to yes, the ImageLoadingTransformer could memorize the width and height of the first image, and check every subsequent image, drop all the samples that don't have the right dimensions, and warn about the records dropped.

cc @yaeldekel @Ivanidzo4ka @TomFinley

[TomFinley]

I'm not sure that I agree this is a good first issue, since it is architecturally thorny.

• if we make a bool about imagesHaveSameDimentions, and that is set to yes, the ImageLoadingTransformer could memorize the width and height of the first image, and check every subsequent image, drop all the samples that don't have the right dimensions, and warn about the records dropped.

At its core, this turns what is currently a relatively straightforward schema-only-mapping (known size image into known size vector) into a "trainable" transformation. (Or not. Whether it's trainable or not depends on a variety of factors.) This by itself is not troublesome -- we have a fair number of estimator/transformer pairs that are trainable or not depending on their settings. But I'd argue they're comparatively difficult to get right for a newcomer.

It also leaves unresolved a few key questions. The big one for me: what does "the records dropped" mean? The most natural interpretation of "dropping a record" to me at least suggests the row is no longer there. Does this mean this is no longer a row mapper? If so, that means it cannot be used in the prediction engine. So that seems questionable. I'd say throw rather than just "dropping" it and writing a warning somewhere. But again, this emphasizes that what is now a simple transformer is now growing a lot in complexity, which brings me to why I might not consider this whole thing a great idea:

• The ImageLoadingEstimator should allow the user to specify the height and width of the images, if the user has images of fixed with and height, rather than fail at "ExtractPixels" if resize is not present.

Present in the proposal is this suggestion to modify an existing estimator/transformer pair. As far as I can see, the only reason why we identified this estimator/transformer pair as the place to add the functionality to is it's the place you saw the error message. That is, it's the place that wants fixed size images. Is that sufficient? I'd say not, because, if we add a second estimator/transformer pair that needs fixed sized images, what then? We'd immediately have to turn around and add it there too, adding complexity, duplicating functionality, etc. etc., which is surely not a good design. (This in fact gets at why we have these composable data pipelines in the first place, is we discovered that it was far, far easier for everyone to actually understand three small simple operations than it is to understand one complex operation.)

What I'd suggest instead, if we really cared, is we have a separate estimator/transform pair that ensures the images are the same size as whatever one it happens to see first, and throws otherwise, this seems fine to me. I would however want that to be a separate estimator/transformer pair, rather than folding it into the transformer. Or else we could change the resizer to make the sizes optional parameters, and infer them from the first image, including throw. Lots of things we could do.

[mareklinka]

This is actually somewhat related to my issue #2022. I realize my understanding of the underlying architecture is not too deep but I'd say having the resizer transform in the pipeline just because the extractor requires it should not be too much of a problem.

1. As mentioned by @TomFinley, each step in the pipeline performs a single, easily understandable task
2. The resizer step does not introduce performance overhead if the image dimensions already match (I think)
3. It makes obvious the later stages of the pipeline require images of certain dimensions AND it makes it's immediately visible what they are, since they need to be provided as parameters

I'm not against having a new "resizer" that just verifies that all input images are of the same dimensions. But having the dims specified explicitly also has its advantages.

[Ivanidzo4ka]

my two cents:

• drop all the samples that don't have the right dimensions, and warn about the records dropped.

How I see 80% of our image transforms been used right now with our API:

1. we build pipeline which process images and shape them all together to certain format.
2. we run pre-train model which has specific restrains on image dimensions.

In this case we need to resizes images anyway. Technically we can push that step on a user, but if we going that way, why not push whole feature preparation on his shoulders? Idea of telling user what you gave us image, but we don't like because width or height is by 1 pixel different, doesn't sound pleasant for me.

I'm all hands up for improving our error message, and suggest user to use ImageResizer to shape image in proper dimensions.