Fix : Falcon processor doesn't account for a layout difference of qkv between transformers and GGUF #35088
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| if model_size == "40b": | ||
| parsed_parameters["config"]["new_decoder_architecture"] = True |
There was a problem hiding this comment.
Can you add tests for the 40b even if we won't run them on the CI (add @Skip) .
There was a problem hiding this comment.
In fact, I'm thinking about avoid using model_size extracted from file name, because some user fine-tuned model may use custom filename without "40b" explicitly. (I also removed the model_size logic in #34385)
Considering "new_decoder_architecture" having 2 layernorm for attn in decoder layer, I will prefer to check the existence of attn_norm_2 to determine "new_decoder_architecture".
You can refer to https://huggingface.co/maddes8cht/tiiuae-falcon-40b-instruct-gguf?show_file_info=tiiuae-falcon-40b-instruct-Q2_K.gguf, which indeed has attn_norm_2 params.
There was a problem hiding this comment.
| if model_size == "40b": | |
| parsed_parameters["config"]["new_decoder_architecture"] = True | |
| new_decoder_architecture = any("attn_norm_2" in tensor.name for tensor in reader.tensors) | |
| parsed_parameters["config"]["new_decoder_architecture"] = new_decoder_architecture |
What does this PR do?
For Falcon 40B in the
transformersmodeling code, the Q, K, and V tensors are fused and stored in an interleaved manner. This means that, for each group, the Q tensors for all heads in the group are stacked together, followed by the K and V matrices of that group. However, in the GGUF layout, thefalcon.tensor_data_layoutis set tojploski, which changes how the fused Q, K, and V tensors are stored (more info here). In this layout, the Q, K, and V tensors are stored sequentially instead of interleaved. This sequential storage makes it easier to use on thellama_cppside. To handle this difference, the PR processes theqkvtensors to convert them back into the interleaved format required fortransformersmodeling.Additionally, this PR adds a new field,
new_decoder_architecture, to the configuration. This is necessary for ensuring the modeling code of Falcon handles thefused_qkvsizes correctly. The PR also fixes the name of thenum_key_value_headsfield in the Falcon configuration, changing it to the correct name,num_kv_heads.Who can review ?
@SunMarc @LysandreJik