Skip to content

fix tokenizers in respect to newer transformers #4608

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

Merged
ericharper merged 18 commits into from
May 14, 2026
Merged

fix tokenizers in respect to newer transformers #4608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ee5ea00
fix tokenizers
dimapihtar May 4, 2026
e01a02e
Merge branch 'main' into fix_transformers_bump
dimapihtar May 4, 2026
9b0aa69
minor fix
dimapihtar May 4, 2026
c8253fe
Merge branch 'main' into fix_transformers_bump
dimapihtar May 5, 2026
0da0924
set return_dict=False
dimapihtar May 5, 2026
95b53e5
minor fix
dimapihtar May 5, 2026
62e86d1
upgrade transformers version
dimapihtar May 5, 2026
565cecb
Revert "upgrade transformers version"
dimapihtar May 5, 2026
dfd187e
upgrade transformers
dimapihtar May 5, 2026
3f28c0c
revert changes
dimapihtar May 5, 2026
a5db619
Merge branch 'main' into fix_transformers_bump
dimapihtar May 5, 2026
7f4120a
switch back to initial solution
dimapihtar May 5, 2026
6c3c93b
Merge branch 'main' into fix_transformers_bump
dimapihtar May 5, 2026
c6361d6
set return_dict=False
dimapihtar May 6, 2026
0af70f8
fix unit test by setting return_dict=False
dimapihtar May 6, 2026
1dcaa77
Merge branch 'main' into fix_transformers_bump
dimapihtar May 6, 2026
734841b
minor change
dimapihtar May 6, 2026
961e804
Merge branch 'main' into fix_transformers_bump
dimapihtar May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion megatron/core/tokenizers/vision/libraries/multimodal_tokenizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ def tokenize_conversation(
add_generation_prompt=add_generation_prompt,
return_assistant_token_mask=False,
return_tensors="np",
return_dict=False,
chat_template=self._prompt_config.custom_chat_template,
)[0]

Expand All @@ -272,7 +273,10 @@ def tokenize_conversation(
raise ValueError(f"empty turn in conversation: {conversation}. Skipping.")

turn_tokens = self.tokenizer.apply_chat_template(
[turn], tokenize=True, chat_template=self._prompt_config.custom_chat_template
[turn],
tokenize=True,
return_dict=False,

@jaredcasper jaredcasper May 6, 2026

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.

How new is the return_dict argument? Are we going to be incompatible with an older but still used transformers version?

@dimapihtar dimapihtar May 7, 2026

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.

@jaredcasper should keep backward compatibility return_dict was False by default in older versions but they changed it to True in 5.3.0+

@dimapihtar dimapihtar May 7, 2026

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.

we used 4.57.3 before this moment AFAIK

chat_template=self._prompt_config.custom_chat_template,
)

# There should be only one BOS at the very beginning.
Expand Down
Loading
Loading