Skip to content

models : fix n_text_ctx taken from max_length instead of max_target_positions - #3987

Open
axiom-of-choice wants to merge 1 commit into
ggml-org:masterfrom
axiom-of-choice:fix-n-text-ctx-from-max-target-positions
Open

models : fix n_text_ctx taken from max_length instead of max_target_positions#3987
axiom-of-choice wants to merge 1 commit into
ggml-org:masterfrom
axiom-of-choice:fix-n-text-ctx-from-max-target-positions

Conversation

@axiom-of-choice

Copy link
Copy Markdown

Closes #3986.

n_text_ctx sizes the decoder positional embedding (src/whisper.cpp:1797), so it must be
max_target_positions, the row count of the decoder.positional_embedding tensor this script
writes. It was reading max_length, a generation default. The audio side of the header already
uses max_source_positions, and convert-h5-to-coreml.py already reads max_target_positions.

Both fields are 448 on OpenAI's checkpoints, so this is invisible until a fine-tune sets
max_length.

Measured with a synthetic checkpoint (max_target_positions=1024, max_length=448) and
whisper-cli built from this tree:

master this change
fine-tune case decoder.positional_embedding has wrong size, load fails loads
OpenAI-style checkpoint loads output .bin byte-identical

#2477 and #2840 used max_target_positions as a fallback for a missing or non-int max_length;
this makes it the source, so both are subsumed. Their coercion is dropped as unreachable:
from_pretrained rejects a non-int, negative or weights-disagreeing value before the header is
written.

Not verified against a real fine-tune. #2778 may be the same bug but has no config.json to
confirm it.

…ositions

n_text_ctx sizes the decoder positional embedding, so it must be the
architecture (max_target_positions), not a generation default. Both are 448 on
OpenAI's checkpoints, so this only shows on a fine-tune that sets max_length,
where the header disagrees with the tensor and the model fails to load.

Subsumes ggml-org#2477 and ggml-org#2840, which used max_target_positions only as a fallback.
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.

convert-h5-to-ggml.py writes n_text_ctx from max_length even when it is present and is not the architecture (follow-up to #2477, may explain #2778)

1 participant