Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vectorize RandomTranslation #1480

Merged
merged 3 commits into from
Mar 10, 2023

Conversation

james77777778
Copy link
Contributor

What does this PR do?

Related to #291

Benchmark results:

Mode n=100 n=200 n=500 n=1000
OldRandomTranslation Eager 0.77s 1.69s 7.84s 30.13s
OldRandomTranslation Graph 0.33s 1.19s 7.41s 29.92s
RandomTranslation Eager 0.0072s 0.0069s 0.0087s 0.0119s
RandomTranslation Graph 0.0014s 0.0019s 0.0042s 0.0077s

comparison

OldRandomTranslation is too slow that I need to set num_images = [100, 200, 500, 1000] for benchmarks

Both implementations fail to run with XLA due to lacking ImageProjectiveTransformV3 support.
tensorflow/tensorflow#55194

Similar to #1435, I add a test to check random translation is applied for each image.

    def test_random_translation_on_batched_images_independently(self):
        image = tf.random.uniform(shape=(100, 100, 3))
        input_images = tf.stack([image, image], axis=0)

        layer = preprocessing.RandomTranslation(
            height_factor=0.5, width_factor=0.5
        )

        results = layer(input_images)
        self.assertNotAllClose(results[0], results[1])

Also, I found that RandomTranslation is missing in keras_cv/layers/preprocessing/ragged_image_test.py and keras_cv/layers/preprocessing/with_labels_test.py. I added it to both tests and they passed on my local env.

    (
        "RandomTranslation",
        layers.RandomTranslation,
        {"height_factor": 0.5, "width_factor": 0.5},
    ),

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

Copy link
Contributor

@LukeWood LukeWood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow - great PR. I'm pretty surprised by just how large the performance delta here is between the vectorized/unvectorized version is in this case (the others seem to be much smaller). Great work!

Copy link
Contributor

@ianstenbit ianstenbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome result -- thanks for the PR!

@ianstenbit
Copy link
Contributor

/gcbrun

@ianstenbit ianstenbit merged commit f9ca22a into keras-team:master Mar 10, 2023
@james77777778 james77777778 deleted the random-translation branch March 11, 2023 02:33
ghost pushed a commit to y-vectorfield/keras-cv that referenced this pull request Nov 16, 2023
* vectorize RandomTranslation

* update ragged_image_test for RandomTranslation

* update with_labels_test for RandomTranslation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants