Skip to content

Conversation

@hainan-xv
Copy link
Collaborator

@hainan-xv hainan-xv commented May 8, 2025

Important

The Update branch button must only be pressed in very rare occassions.
An outdated branch is never blocking the merge of a PR.
Please reach out to the automation team before pressing that button.

What does this PR do ?

Fix issues with TDT buffered inference that results in bad accuracy.

Collection: [Note which collection this PR will affect]

ASR

Changelog

  • change alignment representation of TDT to include skipped frames as well
  • add a boundary matching algo in the TDT buffered inference procedure.

Usage

  • You can potentially add a usage example below
python NeMo/examples/asr/asr_chunked_inference/rnnt/speech_to_text_buffered_infer_rnnt.py \
               ... \
               merge_algo=tdt

GitHub Actions CI

The Jenkins CI system has been replaced by GitHub Actions self-hosted runners.

The GitHub Actions CI will run automatically when the "Run CICD" label is added to the PR.
To re-run CI remove and add the label again.
To run CI on an untrusted fork, a NeMo user with write access must first click "Approve and run".

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you add or update any necessary documentation?
  • Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
    • Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

  • New Feature
  • Bugfix
  • Documentation

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

  • Related to # (issue)

@github-actions github-actions bot added the ASR label May 8, 2025
@hainan-xv
Copy link
Collaborator Author

@nithinraok can you take a look by any chance? So far I'm not sure it's 100% correct yet, since it's noticeably worse than offline inference in my testing. I'm not sure how much we're expecting the gap to be though... Could you let me know if it works OK on your end?

@nithinraok
Copy link
Collaborator

@weiqingw4ng tested it for few samples and it worked great! I will add some numbers here with parakeet-tdt-0.6b-v2 on LS dataset in sometime.

@hainan-xv hainan-xv force-pushed the tdt_buffered_inference_fix branch from 13c3a00 to f8a7ecc Compare May 8, 2025 17:08
@nithinraok nithinraok requested a review from artbataev May 8, 2025 17:13
@nithinraok
Copy link
Collaborator

nithinraok commented May 9, 2025

Below table shows evaluation of same size, same data trained parakeet models with various decoders. Evaluated with greedy (total buffer size, chunk len in sec)

Decoder MODE LS-test clean LS-test other
tdt offline 1.69 3.19
tdt greedy (30,10) 2.07 4.01
tdt greedy (8,0.16) 3.23 5.19
rnnt offline 1.82 3.47
rnnt greedy (30,10) 3.09 5.14
ctc offline 2.2 3.8
ctc greedy (30,10) 2.73 4.95
ctc greedy (8,0.16) 5.39 7.03

Copy link
Collaborator

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test case for buffered tdt inference

# Change Decoding Config
with open_dict(cfg.decoding):
if cfg.stateful_decoding:
if cfg.stateful_decoding or cfg.merge_algo == 'tdt':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this info to the doc string above and also add to script usage at the top of this script.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 9, 2025

# Merge algorithm for transducers
merge_algo: Optional[str] = 'middle' # choices=['middle', 'lcs'], choice of algorithm to apply during inference.
merge_algo: Optional[str] = (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: set merge_algo to None (meaning "auto")
In the code use something like:

if merge_algo is None:
    merge_algo = "tdt" if model.is_tdt() else "middle"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

@artbataev artbataev May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you also set default to None in the config (here)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 9, 2025
@hainan-xv
Copy link
Collaborator Author

Can you add a test case for buffered tdt inference

I can't seem to find test code for the existing buffered RNNT inference to base the new test on. Not sure how to do it efficiently... What I can think of is to include a small trained TDT model in the repo and include a short audio recording also to run this, but this will take quite some work... Shall we do this later?

@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 9, 2025
f14-bertolotti and others added 10 commits May 9, 2025 12:28
Signed-off-by: Francesco Bertolotti <[email protected]>
Co-authored-by: Alexandros Koumparoulis <[email protected]>
Co-authored-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
* fix

Signed-off-by: Alexandros Koumparoulis <[email protected]>

* make fp8 tests non-optional

Signed-off-by: Alexandros Koumparoulis <[email protected]>

* switch to gemma

Signed-off-by: Alexandros Koumparoulis <[email protected]>

---------

Signed-off-by: Alexandros Koumparoulis <[email protected]>
Co-authored-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
* beep boop: Update changelog

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Add changelog highlights

Signed-off-by: Charlie Truong <[email protected]>

---------

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Charlie Truong <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
* tests: Disable flaky test

Signed-off-by: oliver könig <[email protected]>

* remove breakpoint

Signed-off-by: oliver könig <[email protected]>

---------

Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
* Fix 2.3.0 changelog

Signed-off-by: Charlie Truong <[email protected]>

* Update 2.3.0 changelog

Signed-off-by: Charlie Truong <[email protected]>

---------

Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
Signed-off-by: Pranav Prashant Thombre <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
JRD971000 and others added 2 commits May 9, 2025 12:29
* add extra hyena tests

* Apply isort and black reformatting

Signed-off-by: JRD971000 <[email protected]>

* fix num gpus

* keep sft optional

---------

Signed-off-by: JRD971000 <[email protected]>
Co-authored-by: JRD971000 <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
@hainan-xv hainan-xv force-pushed the tdt_buffered_inference_fix branch from 26c24bf to 62284b6 Compare May 9, 2025 16:30
@github-actions github-actions bot added the CI label May 9, 2025
@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 9, 2025
@nithinraok
Copy link
Collaborator

I think something messed up. Now its showing 21 file changes

@github-actions github-actions bot removed the CI label May 10, 2025
@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 10, 2025
@hainan-xv
Copy link
Collaborator Author

I think something messed up. Now its showing 21 file changes

Not sure what happened back then. I did a pull of the latest main and re-push'ed. It seems it's fixed now.

@ko3n1g ko3n1g added Run CICD and removed Run CICD labels May 12, 2025
Copy link
Collaborator

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @hainan-xv

Copy link
Collaborator

@artbataev artbataev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@nithinraok nithinraok merged commit 4749e42 into NVIDIA-NeMo:main May 12, 2025
123 checks passed
shjwudp pushed a commit to shjwudp/NeMo that referenced this pull request May 31, 2025
* added use-fast tokenizer argument (NVIDIA-NeMo#12986)

Signed-off-by: Francesco Bertolotti <[email protected]>
Co-authored-by: Alexandros Koumparoulis <[email protected]>
Co-authored-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* ci: Run selective triggering on dockerfiles and dependencies (NVIDIA-NeMo#13493)

Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* fix buffered inference for tdt

Signed-off-by: Hainan Xu <[email protected]>

* small fixes

Signed-off-by: Hainan Xu <[email protected]>

* [automodel] fallback FP8 + LCE -> FP8 + CE  (NVIDIA-NeMo#13349)

* fix

Signed-off-by: Alexandros Koumparoulis <[email protected]>

* make fp8 tests non-optional

Signed-off-by: Alexandros Koumparoulis <[email protected]>

* switch to gemma

Signed-off-by: Alexandros Koumparoulis <[email protected]>

---------

Signed-off-by: Alexandros Koumparoulis <[email protected]>
Co-authored-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Update changelog for `r2.3.0` (NVIDIA-NeMo#13501)

* beep boop: Update changelog

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Add changelog highlights

Signed-off-by: Charlie Truong <[email protected]>

---------

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Charlie Truong <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Update 2.3.0 changelog (NVIDIA-NeMo#13503)

Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* ci: Remove trt-llm breakpoint (NVIDIA-NeMo#13499)

* tests: Disable flaky test

Signed-off-by: oliver könig <[email protected]>

* remove breakpoint

Signed-off-by: oliver könig <[email protected]>

---------

Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Update 2.3.0 changelog (NVIDIA-NeMo#13504)

* Fix 2.3.0 changelog

Signed-off-by: Charlie Truong <[email protected]>

* Update 2.3.0 changelog

Signed-off-by: Charlie Truong <[email protected]>

---------

Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Enabling flash decode for float16 precision only (NVIDIA-NeMo#13471)

Signed-off-by: Pranav Prashant Thombre <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Fix changelog formatting (NVIDIA-NeMo#13505)

Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Updating the long context performance number for B200 (NVIDIA-NeMo#13468)

* Add without CP numbers for B200 and merge the captioning texts of both into one.

Signed-off-by: Youngeun Kwon <[email protected]>

* figure removed

Signed-off-by: Youngeun Kwon <[email protected]>

---------

Signed-off-by: Youngeun Kwon <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* Autodetect model_type and dtype for deployment using TRT-LLM backend (NVIDIA-NeMo#13209)

* Autodetect model_type and dtype for deployment using TRT-LLM backed

Signed-off-by: Jan Lasek <[email protected]>

* Handling kv_cache_qformat parameter

Signed-off-by: Jan Lasek <[email protected]>

* Apply isort and black reformatting

Signed-off-by: janekl <[email protected]>

* Docstring update

Signed-off-by: Jan Lasek <[email protected]>

---------

Signed-off-by: Jan Lasek <[email protected]>
Signed-off-by: janekl <[email protected]>
Co-authored-by: janekl <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* remove unused variable

Signed-off-by: Hainan Xu <[email protected]>

* Apply isort and black reformatting

Signed-off-by: hainan-xv <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* add doc string, cleaner way of setting mergo_algo

Signed-off-by: Hainan Xu <[email protected]>

* Apply isort and black reformatting

Signed-off-by: hainan-xv <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* add extra hyena tests (NVIDIA-NeMo#13097)

* add extra hyena tests

* Apply isort and black reformatting

Signed-off-by: JRD971000 <[email protected]>

* fix num gpus

* keep sft optional

---------

Signed-off-by: JRD971000 <[email protected]>
Co-authored-by: JRD971000 <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* ci: Add mode files to filter (NVIDIA-NeMo#13517)

Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>

* change default merge_algo for buffered inference to None

Signed-off-by: Hainan Xu <[email protected]>

* Apply isort and black reformatting

Signed-off-by: hainan-xv <[email protected]>

---------

Signed-off-by: Francesco Bertolotti <[email protected]>
Signed-off-by: Hainan Xu <[email protected]>
Signed-off-by: oliver könig <[email protected]>
Signed-off-by: Alexandros Koumparoulis <[email protected]>
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Charlie Truong <[email protected]>
Signed-off-by: Pranav Prashant Thombre <[email protected]>
Signed-off-by: Youngeun Kwon <[email protected]>
Signed-off-by: Jan Lasek <[email protected]>
Signed-off-by: janekl <[email protected]>
Signed-off-by: hainan-xv <[email protected]>
Signed-off-by: JRD971000 <[email protected]>
Co-authored-by: Francesco Bertolotti <[email protected]>
Co-authored-by: Alexandros Koumparoulis <[email protected]>
Co-authored-by: oliver könig <[email protected]>
Co-authored-by: Hainan Xu <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Charlie Truong <[email protected]>
Co-authored-by: pthombre <[email protected]>
Co-authored-by: Youngeun Kwon <[email protected]>
Co-authored-by: Jan Lasek <[email protected]>
Co-authored-by: janekl <[email protected]>
Co-authored-by: hainan-xv <[email protected]>
Co-authored-by: Ali Taghibakhshi <[email protected]>
Co-authored-by: JRD971000 <[email protected]>
Signed-off-by: jianbinc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.