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.

Sign up for GitHub

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

Jump to bottom

Update CLIP to a functional model #2393

Merged
merged 11 commits into from
Apr 10, 2024
Merged

Update CLIP to a functional model #2393

merged 11 commits into from
Apr 10, 2024

Conversation

divyashreepathihalli
Copy link
Collaborator

No description provided.

@tirthasheshpatel
Copy link
Contributor

Is our test suite running with Tf 1.16? I keep getting this error constantly when testing with TensorFlow 1.16.1:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[15], line 1
----> 1 processor = CLIPProcessor(
      2     MODEL_CONFIGS[config_name]["image_resolution"], VOCAB_PATH, MERGE_PATH
      3 )
      4 image = processor.process_images(["two_cats.jpg"])
      5 text_input = ["mountains", "cat on tortoise", "two cats"]

File ~/oss/keras-cv/keras_cv/models/feature_extractor/clip/clip_processor.py:58, in CLIPProcessor.__init__(self, input_resolution, vocabulary, merges, **kwargs)
     51 self.image_transform = self.transform_image
     52 self.tokenizer = CLIPTokenizer(
     53     vocabulary=self.vocabulary,
     54     merges=self.merges,
     55     unsplittable_tokens=["</w>"],
     56 )
     57 self.packer = StartEndPacker(
---> 58     start_value=self.tokenizer.token_to_id("<|startoftext|>"),
     59     end_value=self.tokenizer.token_to_id("<|endoftext|>"),
     60     pad_value=None,
     61     sequence_length=77,
     62     return_padding_mask=True,
     63 )

File ~/oss/virtualenvs/keras-cv-dev/lib/python3.11/site-packages/keras_nlp/src/tokenizers/byte_pair_tokenizer.py:420, in BytePairTokenizer.token_to_id(self, token)
    418 """Convert a string token to an integer id."""
    419 self._check_vocabulary()
--> 420 return self.vocabulary[token]

KeyError: '<|startoftext|>'

@divyashreepathihalli
Copy link
Collaborator Author

divyashreepathihalli commented Mar 19, 2024 •
edited
Loading

Is our test suite running with Tf 1.16? I keep getting this error constantly ...

This needs to be a conditional import, let me send a fix

@divyashreepathihalli
Copy link
Collaborator Author

#2396

@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Mar 19, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Mar 19, 2024
@tirthasheshpatel tirthasheshpatel mentioned this pull request Apr 8, 2024
Merged
@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@tirthasheshpatel tirthasheshpatel force-pushed the fix_clip_jax branch 2 times, most recently from 8fdabd5 to c6e84b9 Compare April 8, 2024 22:40
@tirthasheshpatel
Refactor CLIP to a functional model
3c54540 
update model input format

update golden values

update CLIP to functional model

update tests

code reformat

use dict instead of list

Update keras_cv/models/feature_extractor/clip/clip_model.py

Co-authored-by: Tirth Patel <tirthasheshpatel@gmail.com>

remove build and compute output shape

update model input format

update golden values

Refactor CLIP

Refactor includes:

- CLIPProcessor is now a Keras layer and uses some utilities from KerasNLP to support all types of python types and array inputs
- CLIPImageEncoder, CLIPTextEncoder, and CLIPEncoder now implement a `.compute_output_shape` method (required for CLIP to work with the functional API)
- CLIPHead added to remove raw variables from the CLIP Task models; having variables in `keras.Model` class is tricky since functional API doesn't allow state.
- CLIP checkpointing script has been updated to now work with the new API: new weights will be uploaded to Kaggle.

TODO: attribute KerasNLP wherever relevant
TODO: upload new weights to Kaggle
TODO: refactor the CLIPProcessor class and the CLIP class to also pull tokenizer vocab and merges from Kaggle.

remove build and compute output shape

Some fixes for the refactor

Fix the tests, update presets

update to layers instead of models
@tirthasheshpatel
Copy link
Contributor

In the latest commit, I have removed some numerics tests since we don't match the HF model. Also, updated the presets and squashed everything down to one commit. Sorry, I lost authorship when doing this. I will approve but leave merging up to @divyashreepathihalli since I have a lot of contributions to this. Please let me know if you have any changes in mind, otherwise feel free to merge. Thanks!

tirthasheshpatel
tirthasheshpatel approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@tirthasheshpatel tirthasheshpatel left a comment

Choose a reason for hiding this comment

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

I verified the refactor has the same numerics as the model we had before. But note that those numerics are broken too, we are very far from the equivalent HF model.

keras_cv/models/feature_extractor/clip/clip_model_test.py
Comment on lines +53 to +72
# These values are NOT computing using HF as the reference model.
# Currently, the numerics of the CLIP model don't match the
# HF model exactly (for the same inputs). For the time being,
# these tests just confirm that unrelated changed don't affect
# the numerics. Once the fix for the numerics is in, we can remove
# this comment and the xfail below.
self.assertAllClose(
outputs["image_logits"], [[10.246354, 10.246353, 10.246354]]
)
self.assertAllClose(
text_logits, ops.transpose([[1.896712, 1.896712, 1.896712]])
outputs["text_logits"],
ops.transpose([[10.246354, 10.246353, 10.246354]]),
)

# True reference values computed using HF:
# image_logits: [[17.8013, 17.8013, 17.8013]]
# text_logits: image_logits.T

# xfail after assertion
pytest.xfail("KerasCV CLIP doesn't match the HF model.")
Copy link
Contributor

Choose a reason for hiding this comment

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

@divyashreepathihalli I have included the reference values here computed using the same inputs to the HF model. Currently, xfailing the test to indicate we need to fix this.

@tirthasheshpatel tirthasheshpatel added the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@tirthasheshpatel
Copy link
Contributor

Unrelated Keras 2 failure. @divyashreepathihalli Do we need to the initializer? I am inclined to remove it since we don't yet support training.

@tirthasheshpatel tirthasheshpatel added the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 8, 2024
@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@divyashreepathihalli
Copy link
Collaborator Author

Unrelated Keras 2 failure. @divyashreepathihalli Do we need to the initializer? I am inclined to remove it since we don't yet support training.

yes! we can remove those.

@sampathweb sampathweb added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@tirthasheshpatel tirthasheshpatel added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
Divyashree Sreepathihalli added 2 commits April 9, 2024 20:13
@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@tirthasheshpatel tirthasheshpatel added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@divyashreepathihalli divyashreepathihalli added the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Runs Tests on GPU label Apr 9, 2024
@divyashreepathihalli divyashreepathihalli merged commit c60112e into keras-team:master Apr 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tirthasheshpatel tirthasheshpatel tirthasheshpatel approved these changes

@sampathweb sampathweb Awaiting requested review from sampathweb

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@divyashreepathihalli @tirthasheshpatel @sampathweb @kokoro-team