Added vectorized Auto Contrast implementation.

[sebastian-sz]

What does this PR do?

Modifies AutoContrast to use Vectorized base image layer.

Fixes #1382

Following #1392 I also added the benchmark and numerical check (compare old and new).
Benchmark results:

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue? Please add a link
  to it if that's the case.
• Did you write any new necessary tests?
• If this adds a new model, can you run a few training steps on TPU in Colab to ensure that no XLA incompatible OP are used?

Who can review?

@LukeWood @ianstenbit

[LukeWood]

What does this PR do?

Modifies AutoContrast to use Vectorized base image layer.

Fixes #1382

Following #1392 I also added the benchmark and numerical check (compare old and new). Benchmark results:

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue? Please add a link
  to it if that's the case.
• Did you write any new necessary tests?
• If this adds a new model, can you run a few training steps on TPU in Colab to ensure that no XLA incompatible OP are used?

Who can review?

@LukeWood @ianstenbit

@sebastian-sz any idea why the original is faster in some cases? This is surprising to me, perhaps vectorized-map was performing quite well here?

I assume this is still better when they pass bounding boxes in too though - as it would fallback to map_fn.

[bhack]

@sebastian-sz any idea why the original is faster in some cases? This is surprising to me, perhaps vectorized-map was performing quite well here?

Cause it was never in the fallback list for missing converters and it was not using tf.image OPS:
#291 (comment)

You could already run AutoContrast on master with disabling fallback and enabling warning:

        if self.auto_vectorize:
            return tf.vectorized_map(func, inputs, fallback_to_while_loop=False, warn=True)

[LukeWood]

@sebastian-sz any idea why the original is faster in some cases? This is surprising to me, perhaps vectorized-map was performing quite well here?

Cause it was never in the fallback list for missing converters and it was not using tf.image OPS: #291 (comment)

You could already run AutoContrast on master with disabling fallback and enabling warning:

        if self.auto_vectorize:
            return tf.vectorized_map(func, inputs, fallback_to_while_loop=False, warn=True)

that explains why its the same speed, but I'm still surprised that its faster. Maybe its vectorizing something we didn't realize we could vectorize.

[sebastian-sz]

@bhack @LukeWood
I did inspect both layers, but unfortunately, I cannot pinpoint the place where the difference in execution time comes from.

These differences are not huge (~5ms), yet still, they exist.

[LukeWood]

@bhack @LukeWood I did inspect both layers, but unfortunately, I cannot pinpoint the place where the difference in execution time comes from.

These differences are not huge (~5ms), yet still, they exist.

yeah they're pretty tiny. Also - the one that relies on vectorized_map will use map_fn when bounding boxes are provided, so yours will outperform in this case by a good margin. I'm totally happy w/ this performance personally.

[ianstenbit]

LGTM. Thank you @sebastian-sz!

@LukeWood PTAL and let me know what you think about the a/b testing comment on #1392 as that applies here as well

[ianstenbit]

@sebastian-sz can you please move this test case to the benchmark, instead of this test case?

Then we can merge -- thanks!

[sebastian-sz]

@ianstenbit done. Moved consistency test to benchmarks.

[ianstenbit]

/gcbrun