Skip to content

[Feature] Add --disable-normalize-embedding server arg - #39031

Open
fortunecookiee wants to merge 4 commits into
sgl-project:mainfrom
fortunecookiee:jsheng/embedding-disable-normalize
Open

fortunecookiee wants to merge 4 commits into
sgl-project:mainfrom
fortunecookiee:jsheng/embedding-disable-normalize

Conversation

@fortunecookiee

@fortunecookiee fortunecookiee commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Motivation

Embedding models in SGLang hard-code whether their Pooler L2-normalizes the pooled hidden state, e.g. Pooler(pooling_type=PoolingType.LAST, normalize=True). Serving a checkpoint whose downstream consumer expects un-normalized embeddings (raw pooled vectors, e.g. for a scorer or a retrieval index that does its own normalization) currently requires editing the model file.

This adds a server-level opt-out so the same checkpoint can be served either way without touching model code.

Supersedes #23282, which was auto-closed by the stale bot. This is the same change rebased onto current main and adapted to the arg_groups / runtime-context config layout that landed in the meantime.

Modifications

  • arg_groups/fields/model.py: new disable_normalize_embedding: bool = False field on the model namespace, declared next to is_embedding. The CLI flag --disable-normalize-embedding is generated from the annotation like every other boolean arg.
  • layers/pooler.py: Pooler.__init__ reads the resolved value via get_model() and, when set, overrides normalize=True to False, so the L2 normalization step is skipped at pool time.

Design notes:

  • No per-model change needed. Every model that constructs a Pooler with normalize=True picks up the override automatically.
  • The override is deliberately one-way. A model that already sets normalize=False is unaffected, and there is no path to force-enable normalization on a model that did not opt in. This matches the flag name and avoids surprising behavior changes.
  • Config is read through the model config bag (get_model().disable_normalize_embedding) rather than get_global_server_args(), matching the current resolved-config reader convention (same as the existing get_model().is_embedding reads).

Accuracy Tests

Default path is unchanged: with the flag unset, normalize keeps the value the model passed, so pooled outputs are bit-identical to main.

With --disable-normalize-embedding, the returned embedding is the pre-normalization pooled vector; normalizing it client-side reproduces the default output.

Speed Tests and Profiling

Not applicable — the flag is read once in Pooler.__init__ (construction time), not on the forward path. When set, it removes work (one L2 normalization per pooled batch).

Checklist


CI States

Latest PR Test (Base): 🚫 Run #34899602609
Latest PR Test (Extra): ❌ Run #34899602318
Latest PR Test (AMD ROCm 10): ❌ Run #34899602455

@fortunecookiee

Copy link
Copy Markdown
Contributor Author

Updated with unit tests (test/registered/unit/layers/test_pooler_score_and_pool.py::TestDisableNormalizeEmbedding) covering: the flag disabling normalization for a normalize=True pooler, normalization staying on by default, and the override being one-way. They publish a context via override_server_args, which also pins the fact that Pooler.__init__ now reads the model config bag.

Also clarified in the help text that models whose pooler does not go through Pooler — the cross-encoder, BERT-pooler and vision-pooler paths — are unaffected by the flag.

For context on alternatives considered: --json-model-override-args cannot express this today, because ~35 of the 41 Pooler(...) call sites pass normalize=True as a literal rather than reading anything off the HF config.

@fortunecookiee

Copy link
Copy Markdown
Contributor Author

/tag-run-ci-label

@fortunecookiee

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

Embedding models in SGLang hard-code whether their Pooler L2-normalizes the
pooled hidden state (`Pooler(pooling_type=..., normalize=True)`). Serving a
checkpoint whose downstream consumer expects un-normalized embeddings
currently requires editing the model file.

Add a `--disable-normalize-embedding` server arg. When set, `Pooler.__init__`
overrides `normalize=True` to `False`, so the L2 normalization step is skipped
at pool time. No per-model change is needed: every model that constructs a
`Pooler` with `normalize=True` picks up the override automatically.

The override is deliberately one-way. A model that already sets
`normalize=False` is unaffected, and there is no path to force-enable
normalization on a model that did not opt in. This matches the flag name and
avoids surprising behavior changes.

The field lives on the `model` namespace next to `is_embedding`, and the
pooler reads it through `get_model()` like other resolved-config readers.
- Cover the flag in test_pooler_score_and_pool.py: the override turns off
  normalization for a normalize=True Pooler, leaves it on by default, and is
  one-way (a normalize=False model is never force-normalized). The tests
  publish a context with override_server_args, which also pins the fact that
  Pooler.__init__ now reads the model config bag.
- Say in the help text that models whose pooler does not go through Pooler
  (cross-encoder, BERT pooler, vision poolers) are unaffected by the flag.
@fortunecookiee
fortunecookiee force-pushed the jsheng/embedding-disable-normalize branch from 83b6060 to 45e2f03 Compare September 11, 2026 16:06
@fortunecookiee

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (picks up the test_dsv4_nonpaged_indexer.py fix from #39101, which was the only non-infra failure in the previous run).

fortunecookiee and others added 2 commits September 11, 2026 13:09
Pooler.__init__ read get_model().disable_normalize_embedding whenever
normalize=True. Config bags fail closed until publish/set_server_args
has projected them, so this coupled normalized Pooler construction to a
published runtime config: any standalone/offline path constructing
Pooler(normalize=True) outside a published server context would raise
ValueError("config namespace 'model' not published") where the prior
code touched no global config.

Gate the read on runtime_context.is_config_published("model") and fall
back to the default of keeping normalization when the namespace is
absent -- with nothing published there is no override to honor. The new
helper deliberately skips the role namespace check so a role violation
still raises from the subsequent config_bag read instead of being
reported as unpublished. Production model init runs after publish, so
serving behavior is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant