Skip to content

fix(asr): keep all subsegments in get_subsegment_dict (diarization dataset gen)#15889

Open
harjothkhara wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
harjothkhara:fix/subsegment-dict-drop
Open

fix(asr): keep all subsegments in get_subsegment_dict (diarization dataset gen)#15889
harjothkhara wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
harjothkhara:fix/subsegment-dict-drop

Conversation

@harjothkhara

Copy link
Copy Markdown

What does this PR do ?

Fixes a silent data-loss bug in get_subsegment_dict: all but the last subsegment of each manifest line are dropped, which silently shrinks speaker-diarization training datasets.

Collection: ASR / speaker tasks (nemo/collections/asr/parts/utils/manifest_utils.py)

Changelog

  • In get_subsegment_dict, the two .append(...) calls that record a subsegment's timestamp ('ts') and json entry ('json_dic') were indented at the same level as the enclosing for subsegment in subsegments: loop — i.e. outside it:

    for subsegment in subsegments:
        start, dur = subsegment
    _subsegment_dict[uniq_id]['ts'].append([round(start, deci), round(start + dur, deci)])
    _subsegment_dict[uniq_id]['json_dic'].append(dic)

    So they ran once per manifest line instead of once per subsegment, and start, dur held only the last subsegment — every earlier subsegment was silently discarded. (The for loop was otherwise pointless, which confirms the appends were meant to live inside it.)

  • Fix: indent the two appends into the loop so every subsegment is recorded.

Usage

With window=1.5, shift=0.75, deci=3 on a 5.0s segment, get_subsegments_scriptable yields 6 subsegments:

before (buggy): ts = [[3.75, 5.0]]                       # only the last subsegment
after  (fixed): ts = [[0.0, 1.5], [0.75, 2.25], [1.5, 3.0],
                      [2.25, 3.75], [3.0, 4.5], [3.75, 5.0]]   # all 6

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests? — added tests/collections/asr/utils/test_manifest_utils.py
  • Did you add or update any necessary documentation? — n/a (internal bugfix)
  • Does the PR affect components that are optional to install? — no

PR Type:

  • New Feature
  • Bugfix
  • Documentation

Additional Information

  • Impact: callers create_segment_manifest -> write_truncated_subsegments and scripts/speaker_tasks/create_diarization_train_dataset.py build diarization training data; the bug silently dropped most subsegments per segment (e.g. 5 of every 6 above) with no error.
  • Self-found (no pre-existing issue). Verified GPU-free: the new test drives get_subsegment_dict with a synthetic in-memory manifest and asserts one entry per subsegment (fails on old code, passes on the fix). I don't have a CUDA machine, so pass/fail was confirmed with a standalone replica of the loop; the in-repo test runs as a normal CPU unit test in CI.

In get_subsegment_dict the two `.append(...)` calls that record a subsegment's
timestamp and json entry were indented at the same level as the enclosing
`for subsegment in subsegments` loop, i.e. outside it. As a result they ran
once per manifest line instead of once per subsegment, and `start, dur` held
only the last subsegment -- every earlier subsegment was silently dropped.

This corrupts speaker-diarization training-dataset generation
(create_segment_manifest -> write_truncated_subsegments, and
scripts/speaker_tasks/create_diarization_train_dataset.py): most subsegments
per segment were discarded with no error.

Indent the two appends into the loop so every subsegment is recorded. Adds a
GPU-free unit test that drives get_subsegment_dict with a synthetic manifest
and asserts one entry per subsegment.

Signed-off-by: harjoth <harjoth.khara@gmail.com>
@copy-pr-bot

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

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the waiting-on-maintainers Waiting on maintainers to respond label Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ASR community-request waiting-on-maintainers Waiting on maintainers to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants