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

Fix Segment Anything Model saving bug #2138

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

tirthasheshpatel
Copy link
Contributor

What does this PR do?

Loading keras_cv.models.SegmentAnythingModel presets fail when using Keras 3. This is because Keras 3 moves the random module outside of the ops namespace and one of the layers in the SAM model uses the ops.random.normal function. This PR fixes the error by accessing the function from the top-level keras module (keras.random.normal).

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?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@divyashreepathihalli @sampathweb

@divyashreepathihalli
Copy link
Collaborator

The keras 2 tests could be failing because of our recent changes. I will wait till we sort that out.

@tirthasheshpatel
Copy link
Contributor Author

tirthasheshpatel commented Nov 10, 2023

I think the Keras 2 failures are because of the changes here. Will push a commit to rosolve them. I am surprised the CI passes since, locally, a lot of them are failing because of is_tensor not being present in the Keras 3. Looks like the CI's been using Keras Core so those errors weren't caught! I think everything should be good once keras-team/keras#18756 is in.

@divyashreepathihalli
Copy link
Collaborator

/gcbrun

@divyashreepathihalli
Copy link
Collaborator

I think the Keras 2 failures are because of the changes here. Will push a commit to rosolve them. I am surprised the CI passes since, locally, a lot of them are failing because of is_tensor not being present in the Keras 3. Looks like the CI's been using Keras Core so those errors weren't caught! I think everything should be good once keras-team/keras#18756 is in.

the tests are not yet checking against keras 3. That is why.

@sampathweb sampathweb merged commit 0a6fa79 into keras-team:master Nov 10, 2023
9 checks passed
@tirthasheshpatel tirthasheshpatel deleted the fix-sam branch November 10, 2023 23:35
yuvraj-wale pushed a commit to yuvraj-wale/keras-cv that referenced this pull request Feb 8, 2024
* Fix Segment Anything Model saving bug

* Use keras iitializers for Keras2/3/ compatibility
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.

None yet

3 participants