Skip to content

feat(semdedup): bound pairwise precision and memory - #2318

Draft
praateekmahajan wants to merge 6 commits into
NVIDIA-NeMo:mainfrom
praateekmahajan:praateekm/nmcur-318-semdedup-pairwise-precision
Draft

feat(semdedup): bound pairwise precision and memory#2318
praateekmahajan wants to merge 6 commits into
NVIDIA-NeMo:mainfrom
praateekmahajan:praateekm/nmcur-318-semdedup-pairwise-precision

Conversation

@praateekmahajan

@praateekmahajan praateekmahajan commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements NMCUR-318 on current main:

  • bounds Pairwise similarity memory to a reusable N x B workspace with fixed positive pairwise_batch_size (default 1024);
  • compares every ranked row only with valid earlier-ranked neighbors, preserving deterministic earliest-rank tie behavior;
  • keeps arbitrary ranking columns and the existing id, max_id, and cosine_sim_score output contract;
  • separates KMeans embedding storage precision from Pairwise multiplication precision;
  • supports opt-in FP16 storage/compute while preserving existing FP32 storage and FP32 compute as the defaults;
  • retains FP16 through multiplication/reduction and promotes scores to FP32 only for cuDF output;
  • routes Torch allocations through the RMM resource already used by cuDF/CuPy, without resetting process-global allocator state;
  • clears failed traceback frames so references to large GPU allocations are released;
  • records cumulative footer-scan, read, rank, conversion, compute, and write timings.

The ranked-neighbor sidecar remains an extension point only. This PR does not implement NMCUR-242 or NMCUR-319.

EOS benchmark

Compared PR commit 5e5b8248 with the September 4 main nightly on the 329,774,475-document predict-all workload:

Configuration Workflow Pairwise Duplicates Retained artifacts
Main nightly 1122.8s 740.6s 10,819,452
PR FP32 storage / FP32 compute 786.9s 423.0s 10,819,441 1.025 TB
PR FP32 storage / FP16 compute 539.6s 184.9s 10,823,919 1.024 TB
PR FP16 storage / FP16 compute 497.0s 154.3s 10,823,956 498.7 GB

The bounded FP32 path reduced Pairwise wall time by 42.9% with an 11-document duplicate-count difference. FP16 remains opt-in pending duplicate-ID analysis; the benchmark indicates substantial performance and storage potential.

The benchmark's original per-cluster custom metrics were under-aggregated across executor task batches. Workflow and Pairwise wall times above are valid; this PR now aggregates the granular phase timings per executor batch and removes the redundant row-count and resolved-batch metrics.

The six-entry precision matrix completed successfully for this analysis. The committed nightly YAML now retains only the two existing SemDedup baseline entries, explicitly configured for the FP32 storage/FP32 compute defaults.

An allocator-only EOS A/B is in progress for both committed SemDedup entries:

  • before RMM sharing: pr-2318-2026_09_04__22_24_57_UTC-5d9be38a
  • with RMM sharing: pr-2318-2026_09_04__22_37_45_UTC-fb6bbdbb

A local 330,000-row × 768-dimension Pairwise workload reduced peak GPU memory from 31.83 GiB to 3.49 GiB with RMM sharing; its median compute time was about 7% slower, which is why the EOS A/B is being run before drawing a performance conclusion.

Validation

At fb6bbdbb, using nvcr.io/nvidian/nemo-curator:nightly-2026-09-04 with GPUs 2 and 3 only:

  • Pairwise, KMeans, and semantic-dedup workflow tests: 60 passed, 1 skipped
  • fresh-process RMM allocator and Pairwise functional smoke: passed
  • Ruff check and format check: passed
  • scoped pre-commit checks for changed files: passed

@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.

@github-actions

Copy link
Copy Markdown
Contributor

Comment thread benchmarking/scripts/semdedup_identification_benchmark.py Outdated
Comment thread benchmarking/scripts/semdedup_pairwise_benchmark.py Outdated
Comment thread benchmarking/scripts/semdedup_identification_benchmark.py
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread tests/benchmarking/test_semdedup_identification_benchmark.py Outdated
Comment thread tests/stages/deduplication/semantic/test_pairwise.py Outdated
Comment thread tests/stages/text/deduplication/test_semantic.py Outdated
Comment thread tests/stages/deduplication/semantic/test_pairwise.py Outdated
Comment thread tests/stages/deduplication/semantic/test_workflow.py Outdated
Comment thread tests/stages/deduplication/semantic/test_workflow.py Outdated
Comment thread tests/stages/text/deduplication/test_semantic.py Outdated
Comment thread tests/stages/deduplication/semantic/test_pairwise.py Outdated
Comment thread tests/stages/deduplication/semantic/test_pairwise.py Outdated
@praateekmahajan
praateekmahajan force-pushed the praateekm/nmcur-318-semdedup-pairwise-precision branch from 44c3013 to 5586523 Compare August 18, 2026 00:30
return None


def release_cached_gpu_memory() -> None:

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.

@ayushdg can you review this, to see if it's not sloppy? I'm wondering if we can then also add this in the minhash teardown part

Comment thread tests/stages/deduplication/semantic/test_pairwise.py
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread tests/stages/deduplication/semantic/test_pairwise.py
Comment thread nemo_curator/stages/deduplication/gpu_utils.py Outdated
@praateekmahajan
praateekmahajan force-pushed the praateekm/nmcur-318-semdedup-pairwise-precision branch 2 times, most recently from d63a7f3 to cd18cc1 Compare August 21, 2026 01:31
@praateekmahajan

Copy link
Copy Markdown
Contributor Author

/ok to test cd18cc1

@praateekmahajan

Copy link
Copy Markdown
Contributor Author

/ok to test bcb544b

eps: Epsilon value for duplicate identification threshold (cosine_sim >= 1-eps)
which_to_keep: Strategy for ranking within clusters ("hard", "easy", "random")
pairwise_batch_size: Batch size for pairwise similarity computation
pairwise_compute_dtype: Multiplication precision used by Pairwise

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.

nit: Should we rename this to precision?

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.

Can do it but I think it could become ambiguous.

KMeans outputs FP32 or 16 and this one can perform matmul in FP32 or 16, both of which are "precision" args.

@ayushdg ayushdg 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.

At a high level the concept makes sense to me. Just had high level questions & few nits

Comment thread benchmarking/nightly-benchmark.yaml Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/semantic/pairwise.py Outdated
Comment thread nemo_curator/stages/deduplication/gpu_utils.py Outdated
return None


def release_cached_gpu_memory(*, reset_owned_rmm_pool: bool = False) -> None:

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.

Not an expert on memory stuff but I know there's a way to ask cupy and possibly torch to re-use the same rmm pool being used by cudf. Not sure if we use that or if that needs to be handled separately.

Comment thread benchmarking/nightly-benchmark.yaml Outdated
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
Signed-off-by: Praateek <praateekm@gmail.com>
"""Make Torch allocate from the RMM resource already used by cuDF and CuPy."""
torch.cuda.memory.change_current_allocator(rmm_torch_allocator)

def process(self, task: FileGroupTask) -> FileGroupTask:

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.

Nit but do we still need process if we are doing process_batch now?

except BaseException as exc:
# An exception's traceback otherwise keeps the unwound _process
# frame—and its large GPU objects—alive until after this finalizer.
traceback.clear_frames(exc.__traceback__)

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.

Sorry what is the point of this?

conversion_start = time.perf_counter()
resolved_compute_dtype = _resolve_compute_dtype(cluster_embeddings, self.compute_dtype)
storage_was_converted = cluster_embeddings.dtype != resolved_compute_dtype
cluster_embeddings = cluster_embeddings.to(dtype=resolved_compute_dtype)

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.

So you are doing _resolve_compute_dtype and to(dtype=...) here and then again inside pairwise_cosine_similarity_batched? Are they the same thing each time?

``B``, ``B`` over ``C``, and so on. For example, consider these normalized
four-dimensional embeddings::

X = A [1.00, 0.00, 0.00, 0.00]

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.

This seems like a nice example, is it something we should consider explaining in the docs too?

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