Skip to content

feat(train): hook in max_seq_length to set custom context length; max 131K, default 2K - #22

Closed
paulpak58 wants to merge 2 commits into
Liquid4All:mainfrom
paulpak58:pp/context-len-flag
Closed

feat(train): hook in max_seq_length to set custom context length; max 131K, default 2K#22
paulpak58 wants to merge 2 commits into
Liquid4All:mainfrom
paulpak58:pp/context-len-flag

Conversation

@paulpak58

Copy link
Copy Markdown

No description provided.

@paulpak58
paulpak58 force-pushed the pp/context-len-flag branch from 4fc218d to e63830b Compare August 27, 2026 17:12
@paulpak58
paulpak58 requested a review from mlech26l August 27, 2026 17:41
@paulpak58

Copy link
Copy Markdown
Author

failing lint CI exists on main

Comment thread lqh/tools/definitions.py Outdated
"minimum": 512,
"maximum": 32768,
"description": (
"Max tokens for prompt plus response (default 2048). "

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.

max_seq_length is not a total prompt-plus-response budget for the DPO/GRPO paths this tool also accepts. DPO passes the value to _generate_predictions as max_new_tokens (then caps it at 1024) without accounting for prompt length; GRPO computes max(256, max_seq_length - 512), so the allowed value 512 can consume 768 tokens. A caller lowering this to manage OOM therefore does not get the documented cap. Please either scope the argument to SFT or make the rollout paths enforce the same total budget, with a 512-case test for each supported mode.

Comment thread lqh/tools/handlers.py Outdated
)
lr = learning_rate if learning_rate is not None else recommended.learning_rate
epochs = num_epochs if num_epochs is not None else recommended.num_epochs
seq_len = (

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.

shrink: recommended.training_config() already supplies the default max_seq_length. Delete seq_len and the unconditional dict entry, then override config["training"]["max_seq_length"] only when the argument is non-None. net: -2 lines possible.

@paulpak58 paulpak58 changed the title feat(train): hook in max_seq_length to set custom context length; default 2048 feat(train): hook in max_seq_length to set custom context length; max 131K, default 2K Sep 3, 2026
mlech26l added a commit that referenced this pull request Sep 3, 2026
Text SFT trained at a fixed max_seq_length of 2048 and silently truncated
longer rows. The length is now derived per run from the dataset: the
longest row, rounded up to 1024, capped at 32768. Short datasets keep
their throughput (the calibration probe measures at the configured
length, so a fixed 32k would have collapsed every run to micro-batch 1);
long ones get what they need. Nothing is exposed to the user: no tool
argument, no mention of sequence or context length in the normal flow.

- defaults: MAX_SEQ_LENGTH_CEILING, derived_seq_length(), auto_seq_length
  flag on HParams; DPO/GRPO/vision keep the fixed 2048.
- seq_length (new): submit-time estimate with the base model's
  tokenizer.json via the `tokenizers` wheel (new core dep), character
  fallback when offline (non-ASCII counted one-to-one); streams parquet.
  The backend planner sizes the GPU from the shipped value.
- sft: exact re-measure through the real chat template before the probe,
  capped at the model's max_position_embeddings; rows over the limit are
  dropped with a count (never truncated); counts persisted; all-eval-
  dropped warned. Probe "no_fit" now fails fast with a plain message
  instead of training on the default micro-batch; a skipped probe at
  long context falls back to micro-batch 1.
- calibrate: maybe_autotune_batch_size returns cached|probed|no_fit|skipped.
- Hidden expert override LQH_MAX_SEQ_LENGTH pins the value.
- Skill text, failure messages and the VLM collator error no longer name
  the knob. Unit conftest stubs the Hub tokenizer fetch.

Supersedes PR #22; PR #23 (assistant_only_loss) rebases onto this and
shares tokenized_row_lengths.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015JpgUssR166axwm95srXbg
@paulpak58 paulpak58 closed this Sep 4, 2026
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.

2 participants