Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

[Fix] Inferring a default sequence length during transformers export #1856

Merged
merged 4 commits into from
Nov 29, 2023
Merged

[Fix] Inferring a default sequence length during transformers export #1856

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bd7369
initial commit
dbogunowicz Nov 28, 2023
3cc37c9
Update src/sparseml/transformers/export.py
dbogunowicz Nov 28, 2023
51d68e9
Merge branch 'main' into feature/damian/fix_default_sequence_length
dbogunowicz Nov 29, 2023
1961c28
Merge branch 'main' into feature/damian/fix_default_sequence_length
rahul-tuli Nov 29, 2023
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
14 changes: 12 additions & 2 deletions src/sparseml/transformers/export.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,20 @@ def export_transformer_to_onnx(
)

if sequence_length is None:
if hasattr(config, "max_position_embeddings"):
Copy link

@derekk-nm derekk-nm Nov 28, 2023

Choose a reason for hiding this comment

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

Could a config have both attributes? If so, is max_position_embeddings preferred over max_seq_len, as coded here?

Copy link
Contributor Author

@dbogunowicz dbogunowicz Nov 28, 2023

Choose a reason for hiding this comment

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

Good catch. Regarding both keys being simultaneously present- honestly do not know, I would need to go through more example configs. But preferring max_position_embeddings or max_seq_len sounds reasonable to me.

sequence_length = config.max_position_embeddings
elif hasattr(config, "max_seq_len"):
sequence_length = config.max_seq_len
else:
raise ValueError(
"Could not infer a default sequence length "
"from the HF transformers config. Please specify "
"the sequence length with --sequence_length"
)
_LOGGER.info(
f"Using default sequence length of {config.max_position_embeddings}"
f"Using default sequence length of {sequence_length} "
"(inferred from HF transformers config) "
)
sequence_length = config.max_position_embeddings

tokenizer = AutoTokenizer.from_pretrained(
model_path, model_max_length=sequence_length
Expand Down