Add Phi-4 Backbone #2272
Add Phi-4 Backbone #2272
Conversation
|
I uploaded the output from the KerasHub model, could someone upload a HF version that I can compare and add to the Colab? |
|
P.S. A lot of this code is based on the existing code for Phi-3 (the technical report states it mostly follows the Phi-3-Medium architecture; I simply made the changes from the report and the reference implementation). Should I refactor it to inherit from |
| '`rope_scaling_type` must be `None` or `"su"`.' | ||
| "if `None` is choosed, `RotaryEmbedding` will be used." | ||
| 'if `"su"` is choosed, `Phi4SuScaledRotaryEmbedding` will be ' | ||
| "used." |
There was a problem hiding this comment.
May be change this to --> "rope_scaling_type must be None or su. If None, RotaryEmbedding will be used. If su, Phi4SuScaledRotaryEmbedding will be used."
Add backtick wherever it is necessary.
| vocabulary_size (int): The size of the token vocabulary. Defaults to | ||
| `100_352`. |
There was a problem hiding this comment.
Change this to --> vocabulary_size: int. The size of the token vocabulary. Defaults to `100_352`.
Follow the above arg pattern for others as well, i know this follows same as phi3, but this will be consistent with majority of our models.
| @pytest.mark.extra_large | ||
| def test_all_presets(self): | ||
| for preset in Phi4Backbone.presets: | ||
| self.run_preset_test( | ||
| cls=Phi4Backbone, | ||
| preset=preset, | ||
| input_data=self.input_data, | ||
| ) |
There was a problem hiding this comment.
Usually how big these models will be and how many presets are we testing here?
| hidden_dim=5120, | ||
| intermediate_dim=17_920, | ||
| num_query_heads=40, | ||
| num_key_value_heads=10, | ||
| activation="silu", | ||
| layer_norm_epsilon=1e-5, | ||
| kernel_initializer="glorot_uniform", | ||
| dropout=0, | ||
| max_sequence_length=16_384, | ||
| pretraining_sequence_length=16_384, | ||
| rope_max_wavelength=250_000, |
There was a problem hiding this comment.
Are these default values are mostly common for all the presets in phi-4, if not may be we can remove default values?
| # TODO: Deprecate this in favor of | ||
| # `keras.layers.LayerNormalization(rms_scaling=True)` once Keras 2 support is | ||
| # removed. |
There was a problem hiding this comment.
We don't have keras 2 support now, either update the code or remove/update this comment.
How much similar is Phi-4 compared to Phi-3? What is the approx percentage of code we can reuse? |
|
I guess still Tokenizer and CausalLM and preset file with necessary test files still needs to be added? |
|
Actually, I think we might get away with directly subclassing The only differences are the model name and the defaults; initially I did this copy anticipating architectural changes, but it seems the only ones are in the attention. From the paper's Section 3:
I could not find this sliding window attention in the code, however, so that also remained unchanged, and the tokenizer would be part of the third PR (based on the contributing guidelines). Do you think it's better if I just did that instead? |
Description of the change
This is the first PR in contributing the Phi-4 model to KerasHub, and includes the backbone and its test file.
Reference
Colab Notebook
I've had some trouble getting this part to work, so I need some help. This is my Colab notebook, but the HF model has been pretty annoying to run. On CPU machines, it seems to constantly allocate all available memory (I gave up after giving it 280GB), and on an H200 on Modal, I couldn't get an output after 15 minutes. In the notebook, this line:
at the bottom is the one I have trouble with.
Checklist