Refactor CLIP to a functional model #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tirthasheshpatel
force-pushed
the
clip_refactor
branch
from
40cb646 to
68b327a
Compare
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.
tirthasheshpatel
force-pushed
the
clip_refactor
branch
from
68b327a to
327fc66
Compare
tirthasheshpatel
changed the title
|
The weights load properly now but the numerics are still significantly off for the same inputs. I will make some changes to the tests. Othewrwise the numerics haven't been affected by this PR. If you are happy with it anyways @divyashreepathihalli, feel free to merge this and then merge keras-team#2393 |
|
One more thing, need to update the Kaggle page with the new weights. Will share the presets with you and we can get the Kaggle page updated. @divyashreepathihalli I noticed that the vocab and merges file need to be fetched manually from external urls. We should change that in a way that can enable us to fetch and load the preset for the processor directly from Kaggle. What do you think? |
divyashreepathihalli
merged commit 083481f
into
divyashreepathihalli:fix_clip_jax
4 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates CLIP to use the functional way of writing models in Keras.
It's currently a rough patch since I worked on it a few weeks ago. Will have to polish the overall diff and make sure I am not toughing the numerics.
Refactor includes:
.compute_output_shapemethod (required for CLIP to work with the functional API)keras.Modelclass is tricky since functional API doesn't allow state.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.
@divyashreepathihalli