Skip to content

fix(executor): partition grouped global batches - #2645

Open
sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:mainfrom
sylvesterkaczmarek:fix-global-batch-partitioning-2643
Open

sylvesterkaczmarek wants to merge 1 commit into
NVIDIA:mainfrom
sylvesterkaczmarek:fix-global-batch-partitioning-2643

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Summary

  • keep grouped global-batch operators hash-partitioned even when actor concurrency is 1
  • retain at least the dataset's existing block count so unrelated source groups are not coalesced into one oversized Arrow block
  • preserve single-block behavior for truly ungrouped global-batch operators
  • add regression coverage for existing block counts, actor-pool concurrency, and ungrouped stages

Fixes #2643

Validation

  • python -m compileall -q nemo_retriever/src/nemo_retriever/graph/executor.py nemo_retriever/tests/test_global_batch_partitioning.py
  • git diff --check

@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors data partitioning logic in the execution engine.

The PR appears safe to merge based on the changes in its head-versus-base diff.

Summary

The PR hash-partitions grouped global-batch stages while retaining at least the dataset’s existing block count. Ungrouped stages still use one block. New tests cover existing block counts, actor concurrency, and ungrouped behavior.

Reviews (4) · Last reviewed commit: "fix(executor): partition grouped global ..."

@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix-global-batch-partitioning-2643 branch from c74b10d to b88acbf Compare September 30, 2026 17:26
@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Refreshed this PR onto current upstream today. It is now 0 commits behind and mergeable, with the intended patch preserved and no unresolved review threads. This has been quiet for over two weeks. Could a maintainer please review/merge it, or let me know if anything else is needed?

@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix-global-batch-partitioning-2643 branch 2 times, most recently from b88acbf to dd19873 Compare September 30, 2026 17:40
Refreshed onto current upstream.

Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com>
@sylvesterkaczmarek
sylvesterkaczmarek force-pushed the fix-global-batch-partitioning-2643 branch from dd19873 to c83151f Compare October 1, 2026 07:59
@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Rebased onto current main today; the new GitHub-Verified head is c83151f, 0 behind and mergeable, with no failing checks or unresolved review threads. @nkmcalli, could you review the refreshed head when convenient?

This branch has not been deployed

No deployments
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.

VideoFrameTextDedup global repartition overflows Arrow string offsets at multi-video scale

1 participant