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

Migrating Nnclr to Keras-3[TF BackEnd] #1730

Merged
merged 3 commits into from
Jan 22, 2024
Merged

Conversation

aditya02shah
Copy link
Contributor

This PR migrates the nnclr example to Keras 3.0 as requested in keras-team/keras-cv#2211.

  1. The tutorial has tensorflow dependency for data preprocessing due to tf.data and tf.image.crop_and_resize() function.
  2. Tested the code with torch and jax backends and works fine.

Copy link
Contributor

@divyashreepathihalli divyashreepathihalli left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @aditya02shah !!
My suggestion for tf.image.crop_and_resize() is to use the following two functions in Keras 3

Everything else looks good!

@aditya02shah
Copy link
Contributor Author

@divyashreepathihalli, thank you for your feedback! I considered your suggestion and opted to replace tf.image.crop_and_resize() with the keras_cv.RandomCropAndResize layer, as it more directly aligns with the required functionality. I've tested these changes, and everything else remains intact.
Please review at your convenience. Appreciate your time!

Copy link
Contributor

@divyashreepathihalli divyashreepathihalli left a comment

Choose a reason for hiding this comment

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

LGTM! thank you! please generate the .md and .ipynb files.

@aditya02shah aditya02shah changed the title Migrating Nnclr to Keras-3[All Backends] Migrating Nnclr to Keras-3[TF BackEnd] Jan 21, 2024
@aditya02shah
Copy link
Contributor Author

@divyashreepathihalli I have explicitly set the TensorFlow backend as the default, as utilizing the JAX and PyTorch backends results in errors such as TypeError:can't convert cuda:0 device type tensor to numpy. Use Tensor.cpu() to copy the tensor to host memory first

Copy link
Contributor

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

This example is expected to be TF-only since it uses the GradientTape.

LGTM, thank you!

@fchollet fchollet merged commit fdcafe6 into keras-team:master Jan 22, 2024
3 checks passed
@fchollet
Copy link
Contributor

Actually, it turns out that the files for this example were generated using Keras 2. Can you regenerate using Keras 3?

@aditya02shah
Copy link
Contributor Author

@fchollet I have migrated the pull request here .
There was an issue with some dependencies in my environment but now everything has been fixed.
I sincerely apologize for any inconvenience this may have caused and appreciate your understanding. Thank you for your time and consideration!

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