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

[Port] OD layers to Keras 3 #2295

Merged

Conversation

ariG23498
Copy link
Contributor

@ariG23498 ariG23498 commented Jan 11, 2024

TODOs:

  • roi_align.py
  • roi_generator.py
  • roi_pool.py
  • roi_sampler.py
  • rpn_label_encoder.py

CC: @divyashreepathihalli

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are certain blockers that I need some guidance on:

  • with tf.name_scope("multilevel_crop_and_resize")
  • tf.constant
  • tf.math.divide_no_nan

Choose a reason for hiding this comment

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

What's the blocker exactly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant -- how should I be porting the above mentioned APIs to Keras3.

Choose a reason for hiding this comment

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

I don't think you require name scoping. For constant, use KerasTensor? For third one, just write a custom divide function. Though we should add it in the core actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tips @AakashKumarNain

I am documenting what I did:

  • Removed scopes
  • Used ops.convert_to_tensor or used ops.ones multiplying a scalar to it
  • Use ops.cond for the divide no nan

@ariG23498
Copy link
Contributor Author

@divyashreepathihalli Could you check the port of roi_generator.py? The only thing to note is that the NMS produces -1.0 instead of 0.0 in the bounding boxes (this has been taken care of in the tests).

@divyashreepathihalli
Copy link
Collaborator

Thanks @ariG23498! roi_align and roi_generator LGTM!!

@ariG23498 ariG23498 marked this pull request as ready for review February 3, 2024 05:01
@ariG23498 ariG23498 changed the title [WIP] Porting OD layers to Keras 3 [Port] OD layers to Keras 3 Feb 3, 2024
Copy link
Collaborator

@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.

@divyashreepathihalli
Copy link
Collaborator

Thanks for the updates @ariG23498. There is one failing test

FAILED keras_cv/layers/object_detection/rpn_label_encoder_test.py::RpnLabelEncoderTest::test_rpn_label_encoder_multi_level - AssertionError: 
Not equal to tolerance rtol=1e-06, atol=1e-06
Mismatched value: a is different from b. 
not close where = (array([0]), array([0]))
not close lhs = [0.]
not close rhs = [1.]
not close dif = [1.]
not close tol = [2.e-06]
dtype = float64, shape = (2, 1)
Mismatched elements: 1 / 2 (50%)
Max absolute difference: 1.
Max relative difference: 1.
 x: array([[0.],
       [1.]])
 y: array([[1.],
       [1.]], dtype=float32)
       ```

@ariG23498
Copy link
Contributor Author

Thanks for the updates @ariG23498. There is one failing test

FAILED keras_cv/layers/object_detection/rpn_label_encoder_test.py::RpnLabelEncoderTest::test_rpn_label_encoder_multi_level - AssertionError: 
Not equal to tolerance rtol=1e-06, atol=1e-06
Mismatched value: a is different from b. 
not close where = (array([0]), array([0]))
not close lhs = [0.]
not close rhs = [1.]
not close dif = [1.]
not close tol = [2.e-06]
dtype = float64, shape = (2, 1)
Mismatched elements: 1 / 2 (50%)
Max absolute difference: 1.
Max relative difference: 1.
 x: array([[0.],
       [1.]])
 y: array([[1.],
       [1.]], dtype=float32)
       ```

Hey @divyashreepathihalli do you think this one has to do with the keras.radom API? I am unable to track the source of the failing test.

@ariG23498
Copy link
Contributor Author

@divyashreepathihalli @AakashKumarNain I have created a gist to explain the failing test test_rpn_label_encoder_multi_level

GitHub Gist

One can notice that the tests are flaky due to the radom API for sampling.py. I am unsure why the tests pass in the first place (maybe a PRNG issue). How do you think we should mitigate this?

@divyashreepathihalli
Copy link
Collaborator

@ariG23498 the randomAPI has changed and these tests can be fllaky. Please disable the test and add and issue and add a todo in the code.

@ariG23498
Copy link
Contributor Author

@ariG23498 the randomAPI has changed and these tests can be fllaky. Please disable the test and add and issue and add a todo in the code.

I have made the changes requested. I am also adding the link of the issue to this thread.

#2336

@divyashreepathihalli
Copy link
Collaborator

divyashreepathihalli commented Feb 27, 2024

Please rebase for the fix - #2360 !

removed vectorized map as it was not working for jax and torch
used ops convert_to_numpy in tests to make np operations work on torch tensor
@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Feb 27, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Feb 27, 2024
@divyashreepathihalli
Copy link
Collaborator

Great work on this @ariG23498! this work is very impactful! Thank you!!

@divyashreepathihalli divyashreepathihalli merged commit 9dd547a into keras-team:master Feb 27, 2024
10 checks passed
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