Skip to content

fix(nemotron-omni): accept dense decoder masks - #5181

Closed
jinglinglingling wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
jinglinglingling:linglinj/nemotron-omni-dense-mask-3290
Closed

fix(nemotron-omni): accept dense decoder masks#5181
jinglinglingling wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
jinglinglingling:linglinj/nemotron-omni-dense-mask-3290

Conversation

@jinglinglingling

Copy link
Copy Markdown

Summary

  • distinguish 2D token-validity masks from 4D decoder attention masks during media placeholder detection
  • preserve padding filtering for token masks while avoiding invalid broadcasting on the unpacked NeMo-RL path
  • add a two-sample dense-mask regression test

Test plan

  • ruff check on the changed model and test files
  • ruff format --check on the changed model and test files
  • MBridge unit CI

@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 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.

@jinglinglingling
jinglinglingling force-pushed the linglinj/nemotron-omni-dense-mask-3290 branch from cb2589d to 7bfe219 Compare July 30, 2026 03:41
@jinglinglingling
jinglinglingling force-pushed the linglinj/nemotron-omni-dense-mask-3290 branch from 7bfe219 to 8e1523d Compare July 30, 2026 03:44
@yaoyu-33 yaoyu-33 added area:model Model implementations and HF bridge logic bug Something isn't working community-request needs-review PR is ready for code review and waiting on a reviewer labels Jul 30, 2026
@jinglinglingling
jinglinglingling force-pushed the linglinj/nemotron-omni-dense-mask-3290 branch from 8e1523d to f8976cf Compare August 5, 2026 00:31
Keep media placeholder detection token-aligned when NeMo-RL provides a dense causal mask on the unpacked path.

Signed-off-by: Linglin Jing <linglinj@cw-dfw-cs-001-vscode-01.cm.cluster>
@jinglinglingling
jinglinglingling force-pushed the linglinj/nemotron-omni-dense-mask-3290 branch from f8976cf to 2a3550d Compare August 5, 2026 00:57
@jinglinglingling

Copy link
Copy Markdown
Author

Rebased the dense decoder-mask fix onto current MBridge main at 0314aeae. New PR head: 2a3550d0; DCO and available GitHub checks are passing.

@cuichenx

cuichenx commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

/ok to test 2a3550d

@cuichenx cuichenx added ready-to-merge PR is approved, current, and only waiting for CI to pass before merge and removed needs-review PR is ready for code review and waiting on a reviewer labels Aug 6, 2026
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor

/ok to test 2a3550d

@cuichenx

cuichenx commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@jinglinglingling can you take a look at #5304 which solves the same problem

@cuichenx
cuichenx marked this pull request as draft August 7, 2026 20:53
@aroshanghias-nvd

Copy link
Copy Markdown
Contributor

Superseded by #5304, which has now merged. Closing this duplicate implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:model Model implementations and HF bridge logic bug Something isn't working ready-to-merge PR is approved, current, and only waiting for CI to pass before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants