Skip to content

feat(text): bound vLLM embedding generation - #2317

Merged
praateekmahajan merged 11 commits into
NVIDIA-NeMo:mainfrom
praateekmahajan:praateekm/nmcur-240-semdedup-bounded-embeddings
Aug 21, 2026
Merged

feat(text): bound vLLM embedding generation#2317
praateekmahajan merged 11 commits into
NVIDIA-NeMo:mainfrom
praateekmahajan:praateekm/nmcur-240-semdedup-bounded-embeddings

Conversation

@praateekmahajan

@praateekmahajan praateekmahajan commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Description

Main Change - Add model_inference_batch_size support for vLLM too. Otherwise it generates a gigantic tokenized column for all texts which can result in CPU OOM. Now we only tokenize what we feed into the LLM. The output returned is a pyarrow table (earlier it was a pd.DataFrame with a embedding column which was list[float] i.e. fp64. Now we do fp32.

Also related to OOM: we don't now persist text column always

Other changes

  1. Use create_vllm_llm_with_retry inside setup
  2. Add metadata_fields for embedding (removed text as the default column that gets preserved)

Linear: NMCUR-240

Validation

  • 27 focused embedder, utility, and workflow-configuration tests passed
  • 12 semantic-dedup workflow tests passed across Xenna and Ray Data
  • Pre-commit passed

Benchmark comparison

Compared nightly baseline (ae713b5e) with the PR run (878cf463). Both entries succeeded and processed the same 5,807,132 documents. Time values are seconds; delta is (PR - nightly) / nightly, so negative time deltas are improvements.

Executor Runtime metric Nightly PR Delta
Ray Data time_taken_s 901.925 847.189 -6.07%
Ray Data throughput_docs_per_sec 6,438.60 6,854.59 +6.46%
Ray Data vLLM process_time_mean 2.962 2.850 -3.79%
Ray Data custom tokenization_time_mean 0.388 0.543 +39.78%
Ray Data custom vllm_embedding_time_mean 2.417 2.461 +1.81%
Xenna time_taken_s 768.562 743.618 -3.25%
Xenna throughput_docs_per_sec 7,555.84 7,809.30 +3.35%
Xenna vLLM process_time_mean 3.031 2.910 -4.00%
Xenna custom tokenization_time_mean 0.413 0.560 +35.44%
Xenna custom vllm_embedding_time_mean 2.454 2.510 +2.31%

Overall end-to-end runtime improved by 6.1% on Ray Data and 3.2% on Xenna, while embedding-stage process_time_mean improved by about 4% for both executors. The custom tokenization mean increased, but its overlap with vLLM inference keeps the aggregate stage time lower. This is a single-run comparison on different EOS hosts (eos0288 before and eos0183 after), so modest differences should be treated as directional.

@copy-pr-bot

copy-pr-bot Bot commented Aug 17, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread tests/stages/text/embedders/test_vllm.py Outdated
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
@praateekmahajan
praateekmahajan force-pushed the praateekm/nmcur-240-semdedup-bounded-embeddings branch from 74971eb to b24d3e7 Compare August 17, 2026 23:19
@praateekmahajan
praateekmahajan marked this pull request as ready for review August 17, 2026 23:24
Signed-off-by: Praateek <praateekm@gmail.com>
…aateekm/nmcur-240-semdedup-bounded-embeddings

Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>

@sarahyurick sarahyurick left a comment

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.

LGTM, left one minor minor comment.

Comment thread nemo_curator/stages/text/embedders/vllm.py
Comment thread nemo_curator/stages/text/embedders/vllm.py Outdated
Signed-off-by: Praateek <praateekm@gmail.com>
…-semdedup-bounded-embeddings

Signed-off-by: Praateek <praateekm@gmail.com>
0,
(embedding_matrix.shape[0] + 1) * embedding_matrix.shape[1],
embedding_matrix.shape[1],
dtype=np.int64,

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.

Does this one matter at all?

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.

That was a really good catch 🙏

Before since it was pandas, we didn't have the 2bn list-elements limit, since we moved to pyarrow now, we would've had that limit (iow if embedding_dim=1024, our document batch couldn't have had more than 2mn rows; so i then ended up moving it to ChunkedArray)

This issue isn't really an issue for SemDedup since we're anyway constrained that a single file MUST contain less than 2bn/EMBEDDING_DIM number of rows, however I can imagine there might be some embedding use case out there where we do want to support this

Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>

@VibhuJawa VibhuJawa left a comment

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.

LGTM

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.

3 participants