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 CropAndResize #1439

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

soma2000-lang
Copy link
Contributor

@soma2000-lang soma2000-lang commented Feb 23, 2023

@soma2000-lang soma2000-lang marked this pull request as draft February 23, 2023 06:15
@soma2000-lang soma2000-lang changed the title Mlpmixer Vectorize CropAndResize Feb 23, 2023
@soma2000-lang soma2000-lang force-pushed the mlpmixer branch 2 times, most recently from 839cd0e to 7adadc2 Compare February 23, 2023 14:52
@soma2000-lang soma2000-lang marked this pull request as ready for review February 23, 2023 14:56
def get_random_transformation(
self, image=None, label=None, bounding_box=None, **kwargs
def get_random_transformation_batch(
self, batch_size, label=None, bounding_box=None, **kwargs
Copy link
Contributor

Choose a reason for hiding this comment

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

if not used, removed label and bounding_box arguments

input_resized = tf.image.resize(image, self.target_size)
output = layer(image, training=True)

self.assertNotAllClose(output, input_resized)
Copy link
Contributor

Choose a reason for hiding this comment

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

lets instead do:

self.assertNotAllClose(output[0], output[1])

@LukeWood
Copy link
Contributor

@soma2000-lang looks like the unit tests are failing!

@soma2000-lang
Copy link
Contributor Author

soma2000-lang commented Feb 24, 2023

@LukeWood looking into this.Actually keras-cv is not properly installed in my system because after some system update the pip package did not seem to work.I am trying to fix the problem with pip right now.

@soma2000-lang
Copy link
Contributor Author

soma2000-lang commented Feb 25, 2023

@LukeWood I am trying to fix the pip issue with my system.Till I fix this,

I just run the test using Google Colab and it seems to pass.

Link -https://colab.research.google.com/drive/1-TZpbQDMp1fe3mrfvouWAqH_PISf9SBh?usp=sharing

it will be great if you check it once.

@LukeWood
Copy link
Contributor

Screen Shot 2023-02-25 at 1 46 13 PM

@LukeWood
Copy link
Contributor

@soma2000-lang left a comment including a merge conflict. Can you resolve?

@soma2000-lang
Copy link
Contributor Author

@LukeWood Resolved

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.

almost ready! Please address remaining comments!

@@ -98,7 +98,7 @@ def __init__(
self.force_output_dense_images = True

def get_random_transformation(
self, image=None, label=None, bounding_box=None, **kwargs
self, image=None,**kwargs
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like image is also unused!

@@ -98,7 +98,7 @@ def __init__(
self.force_output_dense_images = True

def get_random_transformation(
self, image=None, label=None, bounding_box=None, **kwargs
self, image=None,**kwargs
Copy link
Contributor

Choose a reason for hiding this comment

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

be sure to run ./shell/format.sh before requesting pull request reviews

@soma2000-lang soma2000-lang force-pushed the mlpmixer branch 2 times, most recently from 9c01ece to dc56bf5 Compare March 1, 2023 02:36
@soma2000-lang soma2000-lang requested a review from LukeWood March 1, 2023 04:28

input_resized = tf.image.resize(image, self.target_size)
output = layer(image, training=True)
output[1] = input_resized
Copy link
Contributor

Choose a reason for hiding this comment

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

does this pass without this line?

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.

by adding:


        input_resized = tf.image.resize(image, self.target_size)
        output = layer(image, training=True)
        output[1] = input_resized

you're no longer testing that the images are independently augmented. Please remove the assignment - if it fails it means the layer is not independently augmenting images.


self.assertAllClose(output, input_resized)

def random_crop_and_resize_on_batched_image_training(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

add test_ prefix or the test wont run

@LukeWood
Copy link
Contributor

LukeWood commented Mar 6, 2023

@soma2000-lang the goal is to independently augment each image. I think getting this to work will require you to update the logic a bit.

@soma2000-lang
Copy link
Contributor Author

soma2000-lang commented Mar 7, 2023

@LukeWood I made required changes.But some tests are still failing!

@@ -188,12 +188,12 @@ def test_augment_one_hot_segmentation_mask(self):
self.assertAllClose(output["segmentation_masks"], input_mask_resized)

def test_augment_bounding_box_single(self):
image = tf.zeros([20, 20, 3])
images = tf.zeros([20, 20, 3])
Copy link
Contributor

Choose a reason for hiding this comment

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

this is actually a single image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sorry ,will do the necessary

@LukeWood
Copy link
Contributor

LukeWood commented Mar 7, 2023

It looks like this is a legitimate failure - can you fix the test failures? @soma2000-lang

@soma2000-lang
Copy link
Contributor Author

soma2000-lang commented Mar 8, 2023

@LukeWood I sent you an email 2 days back .The emIl id starts with seckroll16 .I would be really greatfull if you can check out once.😊

Regards

@soma2000-lang
Copy link
Contributor Author

@james77777778 It would be great if youvcan help me with this.I am stuck on this for long.

@james77777778
Copy link
Contributor

Hi @soma2000-lang
I have roughly read your PR and I can share the idea about self.get_random_transformation_batch

https://colab.research.google.com/drive/15ed5Fqn5zrQClmmuvB1DWrDGwMrHA4Ka?usp=sharing

I think the vectorizing could be complete by modifying self._transform_bounding_boxes and self._crop_and_resize based on batched and independently sampled transformations

@soma2000-lang
Copy link
Contributor Author

@james77777778 Thanks!

@LukeWood
Copy link
Contributor

@soma2000-lang
Copy link
Contributor Author

@LukeWood sure!.Also sorry for the delay I will try to finish this pr soon.

@divyashreepathihalli
Copy link
Collaborator

@soma2000-lang can you please rebase with master?

@soma2000-lang
Copy link
Contributor Author

Sure

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.

4 participants