Skip to content

Add qwen2#28436

Merged
ArthurZucker merged 76 commits into
huggingface:mainfrom
JustinLin610:add_qwen2
Jan 17, 2024
Merged

Add qwen2#28436
ArthurZucker merged 76 commits into
huggingface:mainfrom
JustinLin610:add_qwen2

Conversation

@JustinLin610

Copy link
Copy Markdown
Contributor

Adding Qwen2

This PR adds the support of codes for the coming Qwen2 models. For information about Qwen, please visit https://github.com/QwenLM/Qwen. @ArthurZucker

Comment thread src/transformers/models/qwen2/tokenization_qwen2.py Outdated
Comment thread src/transformers/models/qwen2/tokenization_qwen2.py Outdated
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! My only concern is that the normal attention layer does not use the new argument (max_window_layers, so results might differ between that and the Qwen2FlashAttention2 layer)

Comment thread src/transformers/convert_slow_tokenizer.py Outdated
Comment on lines +89 to +91
# We need to at least pass vocab_file and merges_file to base class
# in case a slow tokenizer needs to be initialized; other can be
# configured through files

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should also define the extra tokens here as well 😉
copying the way we init them in the slow one!

Comment thread tests/models/qwen2/test_tokenization_qwen2.py Outdated
Comment thread src/transformers/models/auto/tokenization_auto.py
Comment thread src/transformers/models/qwen2/modeling_qwen2.py Outdated
return hidden_states.reshape(batch, num_key_value_heads * n_rep, slen, head_dim)


class Qwen2Attention(nn.Module):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this attention class will not really support the max_swa_layers as it only uses the attention mask to perform sliced attention I doubt that it will have the same results no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Warning added for not using flash attention.

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alright, just a final nit on the tokenizer and I think we can merge!
(doc + unk_token for fast)

Comment thread src/transformers/models/qwen2/tokenization_qwen2_fast.py
Comment thread src/transformers/models/qwen2/tokenization_qwen2.py Outdated
Comment thread src/transformers/models/qwen2/tokenization_qwen2_fast.py
Comment thread src/transformers/models/qwen2/tokenization_qwen2_fast.py Outdated
Comment thread src/transformers/models/qwen2/modeling_qwen2.py Outdated
@ArthurZucker

Copy link
Copy Markdown
Collaborator

Thanks a lot for this pr and bearing with me! 🤗

@ArthurZucker
ArthurZucker merged commit d6ffe74 into huggingface:main Jan 17, 2024
@xd2333

xd2333 commented Feb 11, 2024

Copy link
Copy Markdown

Qwen1.5-0.5B-Chat lose output.weight after load from_pretrained and save_pretrained
image
image

@fxmarty

fxmarty commented Feb 27, 2024

Copy link
Copy Markdown
Contributor

SDPA & eager implem don't seem to match on main (5c341d4), even when not using an attn_mask:

FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_0_float16 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 2.188e+00, torch at...
FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_1_bfloat16 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 1.156e+00, torch at...
FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_2_float32 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 1.369e+00, torch at...

@fxmarty

fxmarty commented Feb 27, 2024

Copy link
Copy Markdown
Contributor

cc @JustinLin610 have you tested both code paths?

@YuanzeSun

Copy link
Copy Markdown

Hi, Can I use it on qwen1? If not, how can I adapt it to qwen1? Thank you!

@ydshieh

ydshieh commented Mar 13, 2024

Copy link
Copy Markdown
Collaborator

@JustinLin610 In the tests, Qwen/Qwen2-450m-beta is used in several places, but there is no such repository on Huggingface Hub. Could you let us know which repository you used when adding those tests, please. Thank you in advance.

Currently, this kind implies there is no integration tests (being run) at all when this model was added to transformers.

@JustinLin610

Copy link
Copy Markdown
Contributor Author

I'll fix the code paths. Previously we have tested it and passed all the test. However, we had name changes and caused troubles. Sry about this.

@JustinLin610

Copy link
Copy Markdown
Contributor Author

@JustinLin610 In the tests, Qwen/Qwen2-450m-beta is used in several places, but there is no such repository on Huggingface Hub. Could you let us know which repository you used when adding those tests, please. Thank you in advance.

Currently, this kind implies there is no integration tests (being run) at all when this model was added to transformers.

https://huggingface.co/Qwen/Qwen1.5-0.5B This is the one that corresponds to the original Qwen/Qwen2-450m-beta

@JustinLin610

Copy link
Copy Markdown
Contributor Author

SDPA & eager implem don't seem to match on main (5c341d4), even when not using an attn_mask:

FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_0_float16 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 2.188e+00, torch at...
FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_1_bfloat16 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 1.156e+00, torch at...
FAILED tests/models/qwen2/test_modeling_qwen2.py::Qwen2ModelTest::test_eager_matches_sdpa_inference_2_float32 - AssertionError: False is not true : padding_side=left, use_mask=False, batch_size=1, enable_kernels=False: mean relative difference: 1.369e+00, torch at...

This is strange to me btw as this part of code is exactly copied from the mistral impl.

@JustinLin610

Copy link
Copy Markdown
Contributor Author

Hi, Can I use it on qwen1? If not, how can I adapt it to qwen1? Thank you!

No, you can't use it directly btw. You need to transform the state dict for the adaptation

merges_file,
errors="replace",
unk_token="<|endoftext|>",
bos_token=None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why None?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cross-posting from slack: is it expected that qwen1.5/qwen1.5-moe models have bos_token_id in their config.json, but not in tokenizer_config.json, while bos_token defaults to None in the tokenizer class?

This results in a loaded tokenizer not having a BOS token and I wonder if this is intended

cc @Giuseppe5

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.

8 participants