Add unified prompt model support for multilingual ASR and streaming inference#15666
Add unified prompt model support for multilingual ASR and streaming inference#15666enas-albasiri wants to merge 7 commits into
Conversation
- RNNT-only prompt model (EncDecRNNTBPEModelWithPrompt) and training script - RNNT-only streaming config (fastconformer_transducer_bpe_streaming_prompt.yaml) - Index-based dataset (LhotseSpeechToTextBpeDatasetWithPromptIndex) with per-dataset prompt_mode support (langID/auto/unified) via lhotse input_cfg tags - Backward-compatible hybrid model: accepts both old prompt tensors and new prompt_indices with auto-detection - Streaming inference: set_inference_prompt() + conformer_stream_step() override for both hybrid and RNNT-only models, with target_lang support in standard cache-aware streaming inference script - Config-driven strip_lang_tags in RNNT decoding to remove <xx-XX> tags from output - Remove unused docs/TRAINING_GUIDE.md and hybrid streaming config
Signed-off-by: Enas Albasiri <ealbasiri@nvidia.com>
Signed-off-by: Enas Albasiri <ealbasiri@nvidia.com>
KunalDhawan
left a comment
There was a problem hiding this comment.
Thanks @enas-albasiri! Could you also share some numbers and validate accuracy of training monolingual/multlingual models with the unified-prompt approach?
Adding @pzelasko for review of LhotseSpeechToTextBpeDatasetWithPromptIndex
| ) | ||
| all_hyp_or_transcribed_texts.append(decoded_out[0]) | ||
| best_hyp = None | ||
| else: |
There was a problem hiding this comment.
log_probs is not assigned in the RNNT branch, but is referenced in line 359. Calling conformer_stream_step(..., return_log_probs=True) while cur_decoder == "rnnt" will raise NameError: name 'log_probs' is not defined.
There was a problem hiding this comment.
function conformer_stream_step is rewritten based on link, where log_probs only applies for CTC model. RNNT shouldn't bother.
There was a problem hiding this comment.
The upstream parallel here is what concerns me, in ASRModuleMixin.conformer_stream_step the if return_log_probs: result.append(log_probs) block sits inside the CTC branch where log_probs is defined. This PR moves the same block outside that branch, after the if isinstance(self, EncDecCTCModel) or ...: ... else: ... split. With the current layout, anything that goes down the RNNT else branch (which is the default, self.cur_decoder == "rnnt") and is called with return_log_probs=True will hit NameError: name 'log_probs' is not defined at line 359, it doesn't need a user to do anything unusual, just pass that flag on an RNNT path.
Three equivalent fixes, any one is fine imo:
- Move the
if return_log_probs: block back inside the CTC branch (matches upstream exactly). - Initialize l
og_probs = Noneat the top of the function and let the RNNT path returnNonefor log_probs. if return_log_probs and self.cur_decoder == "rnnt": raise NotImplementedError(...)early in the function.
Same shape exists in the RNNT-only variant at rnnt_bpe_models_prompt.py:247, the parameter is in the signature but completely unused; either drop it from the signature or raise NotImplementedError so callers get a clear signal rather than a silently dropped flag.
There was a problem hiding this comment.
The entire conformer_stream_step function is removed from both rnnt and hybrid prompt model class. As a replacement, I've added a hook function inside mixin.py which calls model-defined _apply_prompt_to_encoded using the existing conformer_stream_step.
|
|
||
| # Strip language-ID tags (e.g. <en-US>) from decoded output. | ||
| # Enable for prompt-conditioned models that emit locale tags after punctuation. | ||
| strip_lang_tags: bool = True |
There was a problem hiding this comment.
This is a global default change that touches every existing RNNT model. The dataclass default (True) is also inconsistent with the runtime self.cfg.get('strip_lang_tags', False) (set in line 332 of this file), depending on whether cfg comes from the dataclass vs YAML, user will get different behavior.
Could you set the dataclass default to False and align both, then opt in only via the new YAML config. This is the safer backward-compatible choice.
There was a problem hiding this comment.
Set strip_lang_tags to False for inference script.
There was a problem hiding this comment.
Thanks! RNNTDecodingConfig.strip_lang_tags: bool = False (line 1872) plus self.cfg.get('strip_lang_tags', False) at line 332 are now consistent, and the streaming-infer script exposes it as an explicit opt-in. Resolving this thread.
One follow-up I'd like to file separately rather than reopening here: the new set_strip_lang_tags method at rnnt_decoding.py:689 only sets self.lang_tag_pattern and not self.strip_lang_tags (the flag the decode path actually reads at line 966), which is why the inference script has to assign both on lines 378–379. I'll leave that as a new inline comment so it doesn't clutter this thread.
There was a problem hiding this comment.
You are right. We will come to that point later for sure as it doesn't break the logic for now. There are also other places worth optimization and consolidation, too.
| trainer=None, | ||
| validate_access_integrity=True, | ||
| ): | ||
| """Delegate to base EncDecRNNTBPEModel to avoid subclass substitution. |
There was a problem hiding this comment.
This restore_from workaround silently delegates to EncDecRNNTBPEModel.restore_from. As highlighted in your comment, restoring a checkpoint that was saved by EncDecRNNTBPEModelWithPrompt via this method will return a plain EncDecRNNTBPEModel, losing prompt_kernel, set_inference_prompt, and the streaming override. Please fix this.
There was a problem hiding this comment.
Losing "prompt_kernel" "set_inference_prompt" "streaming override" will only happen if the model is EncDecRNNTBPEModel class but loaded using EncDecRNNTBPEModelWithPrompt.restore_from(). And it's doesn't matter in this case.
As long as the model is EncDecRNNTBPEModelWithPrompt class, it can be loaded correctly, either loaded using EncDecRNNTBPEModel.restore_from() or EncDecRNNTBPEModelWithPrompt.restore_from(). Or there is other cases where those prompt-specific attr will be lost when loading the prompt model?
| ) | ||
|
|
||
|
|
||
| class TokenizerWrapper: |
There was a problem hiding this comment.
This is similar to TokenizerWrapper in NeMo/nemo/collections/common/tokenizers/aggregate_tokenizer.py. Could you please upstream the desired changes to the exisiting TokenizerWrapper and import it here?
There was a problem hiding this comment.
Done imported TokenizerWrapper instead.
…script, manual enable by setting stip_lang_tags=True at inference(or training) time. 2. Import TokenizerWrapper in dataloader Signed-off-by: Jinhan <jinhanw@nvidia.com>
| from nemo.utils import logging | ||
|
|
||
|
|
||
| class LhotseSpeechToTextBpeDatasetWithPromptIndex(torch.utils.data.Dataset): |
There was a problem hiding this comment.
Why is a separate class needed for this? Can we replace or add an option to the old class instead?
There was a problem hiding this comment.
There is an older version nemo/collections/asr/data/audio_to_text_lhotse_prompt_index.py creating full prompt_tensor from the dataloader, which is heavy-memory. This updated one only returns prompt indices with size [B] (B = batch_size) and model builds the prompt_tensor. This updated one will be used later on. Will work on to remove the older one.
| encoded = self.prompt_kernel(torch.cat([encoded, prompt], dim=-1)).to(out_dtype) | ||
| return encoded.transpose(1, 2) # (B, T, D) -> (B, D, T) | ||
|
|
||
| def conformer_stream_step( |
There was a problem hiding this comment.
Should it be encoder_stream_step? Why hardcode the architecture?
There was a problem hiding this comment.
Added a hook function in nemo/collections/asr/parts/mixins/mixins.py. Removed conformer_stream_step from both classes.
| # setting the RNNT decoder as the default one | ||
| self.cur_decoder = "rnnt" | ||
|
|
||
| # Streaming inference with language-ID prompt |
There was a problem hiding this comment.
Looks to me like the methods in this section could just as well be a Mixin class PromptableRNNTMixin to avoid duplication in hybrid/non-hybrid model definition
| # Update the joint fused batch size or disable it entirely if needed. | ||
| self.update_joint_fused_batch_size() | ||
|
|
||
| def set_strip_lang_tags(self, strip_lang_tags: bool): |
There was a problem hiding this comment.
this method should also accept the regex pattern, with the current one as default if set to None
| # Field to use for prompt key (default to 'target_lang') | ||
| self.prompt_field = cfg.get('prompt_field', 'target_lang') | ||
|
|
||
| self.training_mode = cfg.get('training_mode', True) |
There was a problem hiding this comment.
Can we remove this and add default_prompt_mode: "inference" instead? One less option.
| manifest_filepath: ??? | ||
| sample_rate: ${model.sample_rate} | ||
| use_lhotse: true | ||
| shard_manifests: true |
There was a problem hiding this comment.
this option doesn't exist in lhotse dataloader
| min_duration: 0.1 | ||
| is_tarred: true | ||
| tarred_audio_filepaths: null | ||
| shuffle_n: 2048 |
There was a problem hiding this comment.
this option doesn't exist in lhotse dataloader
| tarred_audio_filepaths: null | ||
| shuffle_n: 2048 | ||
| slice_length: 100 | ||
| bucketing_strategy: "fully_randomized" |
There was a problem hiding this comment.
this option doesn't exist in lhotse dataloader
| batch_duration: null | ||
| use_lhotse: true | ||
| use_bucketing: false | ||
| max_cuts: 8 |
There was a problem hiding this comment.
this option doesn't exist in lhotse dataloader (use batch_size)
| sample_rate: ${model.sample_rate} | ||
| batch_size: 2 | ||
| shuffle: false | ||
| use_start_end_token: false |
There was a problem hiding this comment.
this option doesn't exist in lhotse dataloader (unless you pass it onto the dataset class?)
…nnt prompt model yaml Signed-off-by: Jinhan <jinhanw@nvidia.com>
| # Update the joint fused batch size or disable it entirely if needed. | ||
| self.update_joint_fused_batch_size() | ||
|
|
||
| def set_strip_lang_tags(self, strip_lang_tags: bool): |
There was a problem hiding this comment.
Currently this method only sets self.lang_tag_pattern (and only when called with True). It does not touch self.strip_lang_tags, which is the flag the decode path actually reads indecode_tokens_to_str_with_strip_punctuation at line 966 (if self.strip_lang_tags: ...). That's why the streaming-infer script has to do this two-step at lines 378–379:
asr_model.decoding.strip_lang_tags = cfg.strip_lang_tags # set the flag
asr_model.decoding.set_strip_lang_tags(cfg.strip_lang_tags) # also compile the pattern
Could we either (a) have the method set both the flag and the pattern in one shot:
def set_strip_lang_tags(self, strip_lang_tags: bool):
self.strip_lang_tags = strip_lang_tags
if strip_lang_tags and not hasattr(self, 'lang_tag_pattern'):
self.lang_tag_pattern = re.compile(r'\s*<[a-z]{2}-[A-Z]{2}>')
so the caller becomes asr_model.decoding.set_strip_lang_tags(cfg.strip_lang_tags). Or (b) drop the method entirely, move the regex compile out of the if block in __init__ (line 333) so it's always available, and let callers just assign the attribute? Either way, the regex literal at line 334 and line 692 is currently duplicated, would be good to lift it to a module-level constant.
…ty fallback map - context_manager.update_cache now `.to(dtype=...)`-coerces the encoder's new cache tensors to the pre-allocated cache buffer's dtype before index_copy_. When TRT's io_dtype_overrides doesn't take effect, the engine emits fp32 cache outputs even though apply_fp16_cache_patch allocated fp16 buffers; without the cast we crash on `index_copy_(): self and source expected to have the same dtype`. When dtypes match the cast is a no-op. - Empty _NEMO_TARGET_FALLBACKS: the entry mapping the missing rnnt_bpe_models_prompt.EncDecRNNTBPEModelWithPrompt to the hybrid class is obsolete now that the real class exists on this branch (cherry-picked from PR NVIDIA-NeMo#15666). The peek-target-from-config mechanism stays in place as a safety net for future missing classes.
Important
The
Update branchbutton 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 ?
Add unified prompt architecture for multilingual ASR, enabling language-ID conditioned for both Hybrid RNNT-CTC and RNNT models as well as streaming inference support.
Collection: ASR
Changelog
EncDecRNNTBPEModelWithPrompt— RNNT prompt-conditioned model with training script and 600M streaming configLhotseSpeechToTextBpeDatasetWithPromptIndex— index-based Lhotse dataset with per-dataset prompt mode support (langID/auto/unified) via lhotseinput_cfgtagsforward()in hybrid model accepting both legacy prompt tensors[B, T, D]and new prompt indices[B]set_inference_prompt()+conformer_stream_step()override for streaming inference in both model variantstarget_langparameter tospeech_to_text_cache_aware_streaming_infer.py(defaults to"auto"for prompt models)strip_lang_tagsconfig inRNNTDecodingConfigto remove<xx-XX>locale tags from decoded outputUsage
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:
PR Type:
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