Audio: add NeMo FastConformer ASR adapter - #2254
Conversation
6e97d31 to
42b7cb6
Compare
c8e6c58 to
a54f655
Compare
Greptile SummaryThe PR introduces a NeMo FastConformer adapter and migrates in-repository NeMo ASR usage to the shared
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported legacy-import concern is addressed by the explicitly intentional breaking migration and accompanying guidance. Important Files Changed
Sequence DiagramsequenceDiagram
participant Runner as Pipeline Runner
participant Stage as ASRStage
participant Adapter as NeMoASRAdapter
participant NeMo as NeMo ASRModel
Runner->>Stage: setup_on_node()
Stage->>Adapter: download_weights_on_node()
Runner->>Stage: setup()
Stage->>Adapter: load_model(num_gpus)
Runner->>Stage: process_batch(AudioTask[])
Stage->>Stage: decode, mono-convert, resample, filter
Stage->>Adapter: transcribe_batch(items)
Adapter->>NeMo: transcribe(waveforms)
NeMo-->>Adapter: hypotheses/text
Adapter-->>Stage: ASRResult[]
Stage-->>Runner: updated AudioTask[]
Runner->>Stage: teardown()
Stage->>Adapter: unload_model()
Reviews (12): Last reviewed commit: "Fix ASR adapter consistency after rebase" | Re-trigger Greptile |
a890d75 to
e6329f8
Compare
| _model: Any = field(default=None, init=False, repr=False) | ||
|
|
||
| def __post_init__(self) -> None: | ||
| if not self.model_id: |
There was a problem hiding this comment.
Some of these checks are overkill IMO. Like this and the check for whether enable_local_attention (and others) is a boolean.
| msg = f"NeMoASRAdapter requires num_gpus to be 0 or 1, got {num_gpus!r}" | ||
| raise ValueError(msg) | ||
|
|
||
| import torch |
There was a problem hiding this comment.
Let's use top-level imports for most of these.
There was a problem hiding this comment.
Let's move all the tests to test_nemo_asr.
| ```bash | ||
| python nemo_curator/config/run.py \ | ||
| --config-path ../../tutorials/audio/nemo_fastconformer \ | ||
| --config-name pipeline \ |
There was a problem hiding this comment.
Can config-name be optional?
| "nemo_toolkit[asr]>=2.7.2; platform_machine == 'x86_64' and platform_system != 'Darwin'", | ||
| "soundfile>=0.12.0", | ||
| "torchaudio", | ||
| "torchcodec==0.11.1; platform_machine == 'x86_64' and platform_system != 'Darwin'", |
c414de6 to
9e73313
Compare
| } | ||
| }, | ||
| "outputs": [], | ||
| "source": [ | ||
| "configure_loguru()\n", | ||
| "ray_client.stop()" | ||
| "configure_loguru()\nray_client.stop()" |
There was a problem hiding this comment.
Just wondering why do we need configure_loguru here?
| explicit = true | ||
|
|
||
| [[tool.uv.index]] | ||
| name = "pytorch-cpu" |
There was a problem hiding this comment.
I was also wondering why it needs this?
a395afd to
9e38391
Compare
|
/ok to test 9e38391 |
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
|
/ok to test 9a7ef2c |
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
|
/ok to test 16a0c81 |
Summary
Adds a NeMo ASR adapter for the shared
ASRStage/ASRAdaptercontract and migrates Curator's in-repository NeMo ASR consumers to that adapter-based path.The resulting ownership split is:
ASRStageowns file decoding, mono conversion, resampling, language filtering, batching, skip handling, output assembly, resources, and lifecycle routing.NeMoASRAdapterowns NeMo checkpoint download, worker-local model load/unload, optional FastConformer local-attention conversion, optional RNNT CUDA-graph decoder configuration, batched transcription, and NeMo output normalization.Changes
NeMoASRAdapterbehind the shared ASR adapter protocol.adapter_kwargsrather than forcing them onto every ASR adapter.ASRStage+NeMoASRAdapter.README.md+pipeline.yaml.pipelineis the default Hydra config name.main.Intentional migration
This PR removes the legacy
nemo_curator.stages.audio.inference.asr.asr_nemo.InferenceAsrNemoStagemodule and migrates all in-repository consumers to the shared stage/adapter path. It does not add a compatibility re-export; downstream users of the legacy import must move to:Review feedback addressed
torchand OmegaConf imports to module scope while retaining the optional NeMo lazy import.tests/models/asr/test_nemo_asr.py.--config-name pipelineoptional for shared-runner tutorials and added CLI coverage.Validation after rebasing onto current
mainuv lock --checkpassed with uv 0.12.2.git diff --checkpassed.