NeMo + Lhotse integration #7880

pzelasko · 2023-11-13T18:27:50Z

What does this PR do ?

This PR adds an option to leverage Lhotse for dataloading in NeMo.

Collection: Currently ASR only, with clear path to extend into other collections.

Changelog

Support creating a Lhotse dataloader from NeMo configuration
Support for most Lhotse-specific dataloading features, especially: dynamic bucketing, multiplexing datasets.
Support initializing Lhotse dataloader either from Lhotse manifests, Lhotse Shar (tarred) manifests, NeMo manifests, and NeMo tarred manifests
ASR dataloading for hybrid CTC transducer as the first example

Usage

See the unit tests
NeMo's model.{train,validation,test}_ds sections can be extended with additional fields use_lhotse: True/False and lhotse: {...} to enable this.

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)

nithinraok · 2023-11-21T19:18:56Z

jenkins

tests/collections/asr/test_lhotse_dataloading.py

nemo/collections/common/data/lhotse/nemo_adapters.py

tests/collections/asr/test_lhotse_dataloading.py

github-actions · 2023-12-30T01:44:58Z

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

nemo/collections/asr/models/transformer_bpe_models.py

+            from nemo.collections.asr.data.audio_to_text_lhotse import LhotseSpeechToTextBpeDataset
+            from nemo.collections.common.data.lhotse import get_lhotse_dataloader_from_config
+
+            return get_lhotse_dataloader_from_config(


tests/collections/common/test_lhotse_dataloading.py

Signed-off-by: Piotr Żelasko <[email protected]>

titu1994

Overall it looks quite good to me. There are minor changes required

Change all copyright header in your PR to 2024
Move all hard dependency imports to the top of the file and not inside functions or classes
Move all soft dependency imports under try catch pattern, see numba and apex for reference in NeMo

Final review @VahidooX - from my side this is ok to merge with above fixes

titu1994 · 2024-01-09T18:14:46Z

Dockerfile

@@ -82,13 +82,15 @@ RUN INSTALL_MSG=$(/bin/bash /tmp/torchaudio_build/scripts/installers/install_tor

 # install nemo dependencies
 WORKDIR /tmp/nemo
+ENV LHOTSE_REQUIRE_TORCHAUDIO=0


Is this needed?

Yes, it prevents Lhotse from pulling/checking torchaudio.

Does the user need to set it when not using the dockers?

Only if they can't have torchaudio installed for some reason. IIUC the issue with torchaudio mostly stems from upstream docker images not having it pre-built and the need to build from source when building NeMo containers (which may fail for a number of reasons). But outside these docker images it's straightforward to pip/conda install torchaudio alongside torch. WDYT?

titu1994 · 2024-01-09T18:15:03Z

Dockerfile

 COPY requirements .
 RUN for f in $(ls requirements*.txt); do pip3 install --disable-pip-version-check --no-cache-dir -r $f; done

 # install flash attention
 RUN pip install flash-attn
 # install numba for latest containers
 RUN pip install numba>=0.57.1
+RUN pip install pyloudnorm


Remove from here, put in requirements for asr.txt

titu1994 · 2024-01-09T18:15:45Z

docs/source/asr/datasets.rst

+    model.train_ds.shuffle=true                 # optional
+
+    # Lhotse dataloading related arguments
+    +model.train_ds.use_lhotse=True


Use ++ to be safer (in case key already exists in config)

titu1994 · 2024-01-09T18:17:08Z

nemo/collections/asr/data/audio_to_text_lhotse.py

+        }
+
+    def __init__(self, tokenizer):
+        from lhotse.dataset import AudioSamples


Put these imports in try catch at the top, after the nemo imports. See other examples of import guards.

IMO its better to know what imports are there in a file at the top rather than hunt down imports in file.

titu1994 · 2024-01-09T18:18:04Z

nemo/collections/asr/data/audio_to_text_lhotse.py

+    """
+
+    def __init__(self, tokenizer):
+        from nemo.collections.common.tokenizers.aggregate_tokenizer import AggregateTokenizer


Why are these imported here? They arent problematic.

titu1994 · 2024-01-09T18:18:45Z

nemo/collections/asr/models/ctc_bpe_models.py

@@ -90,6 +90,17 @@ def __init__(self, cfg: DictConfig, trainer=None):
        )

    def _setup_dataloader_from_config(self, config: Optional[Dict]):
+        if config.get("use_lhotse"):
+            from nemo.collections.asr.data.audio_to_text_lhotse import LhotseSpeechToTextBpeDataset


Put these imports at the top of the file. If they are guarded properly there's no issue in importing the file.

titu1994 · 2024-01-09T18:19:54Z

nemo/collections/common/data/lhotse/__init__.py

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from .cutset import read_cutset_from_config


No relative imports - please use absolute imports throughout NeMo codebase

titu1994 · 2024-01-09T18:22:09Z

nemo/collections/common/data/lhotse/dataloader.py

+    """
+    logging.info("We will be using a Lhotse DataLoader.")
+
+    from lhotse import CutSet


Import at top with try catch, check inside function if HAVE_LHOTSE

titu1994 · 2024-01-09T18:23:45Z

nemo/collections/common/data/lhotse/nemo_adapters.py

+
+try:
+    from lhotse.lazy import ImitatesDict
+except ImportError:


catch both ImportError and ModuleNotFoundError (some python versions raise the latter)

requirements/requirements_asr.txt

Signed-off-by: Piotr Żelasko <[email protected]>

pzelasko · 2024-01-09T20:04:35Z

@titu1994 all suggestions implemented

Signed-off-by: Piotr Żelasko <[email protected]>

…sko/nemo into feature/lhotse-integration

Signed-off-by: Piotr Żelasko <[email protected]>

titu1994

Overall looks great, just need @VahidooX's final signoff

titu1994 · 2024-01-09T23:57:58Z

Need to pass jenkins though

pzelasko · 2024-01-10T13:48:19Z

jenkins

VahidooX

Lgtm generally, just some minor comments!

VahidooX · 2024-01-11T22:00:49Z

Dockerfile

@@ -82,13 +82,15 @@ RUN INSTALL_MSG=$(/bin/bash /tmp/torchaudio_build/scripts/installers/install_tor

 # install nemo dependencies
 WORKDIR /tmp/nemo
+ENV LHOTSE_REQUIRE_TORCHAUDIO=0


Does the user need to set it when not using the dockers?

docs/source/asr/datasets.rst

VahidooX · 2024-01-11T22:07:13Z

nemo/collections/asr/data/audio_to_text_lhotse.py

+        return self._tokenizer(text)
+
+
+def _identity(x):


Where do you use this function?

Good catch, not needed anymore. I removed it.

requirements/requirements_asr.txt

Signed-off-by: Piotr Żelasko <[email protected]>

pzelasko · 2024-01-12T14:25:33Z

jenkins

titu1994

Lgtm, thanks for the awesome work !

pzelasko · 2024-01-13T00:24:13Z

Thanks guys! Such a joy to click merge after all this work 😀

* Lhotse integration squashed PR Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Som Signed-off-by: Piotr Żelasko <[email protected]> * Update copyright headers to 2024 Signed-off-by: Piotr Żelasko <[email protected]> * Fix NLP imports Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Vahid Signed-off-by: Piotr Żelasko <[email protected]> --------- Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Piotr Żelasko <[email protected]>

* Lhotse integration squashed PR Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Som Signed-off-by: Piotr Żelasko <[email protected]> * Update copyright headers to 2024 Signed-off-by: Piotr Żelasko <[email protected]> * Fix NLP imports Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Vahid Signed-off-by: Piotr Żelasko <[email protected]> --------- Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Sasha Meister <[email protected]>

* Lhotse integration squashed PR Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Som Signed-off-by: Piotr Żelasko <[email protected]> * Update copyright headers to 2024 Signed-off-by: Piotr Żelasko <[email protected]> * Fix NLP imports Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Vahid Signed-off-by: Piotr Żelasko <[email protected]> --------- Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Pablo Garay <[email protected]>

* Lhotse integration squashed PR Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Som Signed-off-by: Piotr Żelasko <[email protected]> * Update copyright headers to 2024 Signed-off-by: Piotr Żelasko <[email protected]> * Fix NLP imports Signed-off-by: Piotr Żelasko <[email protected]> * Code review - Vahid Signed-off-by: Piotr Żelasko <[email protected]> --------- Signed-off-by: Piotr Żelasko <[email protected]> Signed-off-by: Piotr Żelasko <[email protected]>

github-actions bot added the ASR label Nov 13, 2023

github-advanced-security bot found potential problems Nov 21, 2023

View reviewed changes

tests/collections/asr/test_lhotse_dataloading.py Fixed Show fixed Hide fixed

github-actions bot added the common label Nov 22, 2023

pzelasko force-pushed the feature/lhotse-integration branch from ff22273 to 5f11fdb Compare December 12, 2023 16:03

pzelasko marked this pull request as ready for review December 13, 2023 22:48

github-advanced-security bot found potential problems Dec 14, 2023

View reviewed changes

nemo/collections/common/data/lhotse/nemo_adapters.py Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Dec 15, 2023

View reviewed changes

tests/collections/asr/test_lhotse_dataloading.py Fixed Show fixed Hide fixed

github-actions bot added the CI label Dec 15, 2023

github-actions bot added the stale label Dec 30, 2023

pzelasko removed the stale label Jan 2, 2024

github-advanced-security bot found potential problems Jan 2, 2024

View reviewed changes

tests/collections/common/test_lhotse_dataloading.py Fixed Show fixed Hide fixed

pzelasko force-pushed the feature/lhotse-integration branch from 0f9ed2f to 36a7bda Compare January 3, 2024 14:39

github-actions bot added core Changes to NeMo Core TTS NLP Speaker Tasks labels Jan 3, 2024

pzelasko force-pushed the feature/lhotse-integration branch from 36a7bda to 0f9ed2f Compare January 3, 2024 14:44

github-actions bot removed core Changes to NeMo Core TTS NLP Speaker Tasks labels Jan 3, 2024

pzelasko force-pushed the feature/lhotse-integration branch from 355f5dc to abe3fcb Compare January 8, 2024 15:09

github-actions bot added TTS NLP labels Jan 8, 2024

pzelasko force-pushed the feature/lhotse-integration branch from abe3fcb to 1f23eee Compare January 8, 2024 15:11

github-actions bot removed TTS NLP labels Jan 8, 2024

Lhotse integration squashed PR

252fd43

Signed-off-by: Piotr Żelasko <[email protected]>

pzelasko force-pushed the feature/lhotse-integration branch from 3394c67 to 252fd43 Compare January 8, 2024 15:15

titu1994 reviewed Jan 9, 2024

View reviewed changes

Code review - Som

0c31f27

Signed-off-by: Piotr Żelasko <[email protected]>

pzelasko force-pushed the feature/lhotse-integration branch from e3507bc to 0c31f27 Compare January 9, 2024 20:03

Merge branch 'main' into feature/lhotse-integration

b85a4c5

pzelasko and others added 3 commits January 9, 2024 15:06

Update copyright headers to 2024

ffcf5d4

Signed-off-by: Piotr Żelasko <[email protected]>

Merge branch 'feature/lhotse-integration' of https://github.com/pzela…

ebc0dea

…sko/nemo into feature/lhotse-integration

Fix NLP imports

b3cfc8a

Signed-off-by: Piotr Żelasko <[email protected]>

titu1994 reviewed Jan 9, 2024

View reviewed changes

VahidooX requested changes Jan 11, 2024

View reviewed changes

pzelasko added 2 commits January 12, 2024 09:22

Code review - Vahid

bcac686

Signed-off-by: Piotr Żelasko <[email protected]>

Merge branch 'main' into feature/lhotse-integration

24fbf53

titu1994 approved these changes Jan 12, 2024

View reviewed changes

VahidooX approved these changes Jan 13, 2024

View reviewed changes

pzelasko merged commit 199a8ba into NVIDIA:main Jan 13, 2024
11 checks passed

kobenaxie mentioned this pull request Jan 15, 2024

IO 重构，提升多机多卡训练效率 + 代码复用 wenet-e2e/wenet#2152

Closed

kobenaxie mentioned this pull request Feb 19, 2024

support for large scale data training(millions or even tens of millions of hours)? k2-fsa/icefall#1505

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NeMo + Lhotse integration #7880

NeMo + Lhotse integration #7880

pzelasko commented Nov 13, 2023 •

edited

Loading

nithinraok commented Nov 21, 2023

github-actions bot commented Dec 30, 2023

titu1994 left a comment

titu1994 Jan 9, 2024

pzelasko Jan 9, 2024

VahidooX Jan 11, 2024

pzelasko Jan 13, 2024 •

edited

Loading

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

titu1994 Jan 9, 2024

pzelasko commented Jan 9, 2024

titu1994 left a comment

titu1994 commented Jan 9, 2024

pzelasko commented Jan 10, 2024

VahidooX left a comment

VahidooX Jan 11, 2024

VahidooX Jan 11, 2024

pzelasko Jan 12, 2024

pzelasko commented Jan 12, 2024

titu1994 left a comment

pzelasko commented Jan 13, 2024

NeMo + Lhotse integration #7880

NeMo + Lhotse integration #7880

Conversation

pzelasko commented Nov 13, 2023 • edited Loading

What does this PR do ?

Changelog

Usage

Before your PR is "Ready for review"

Who can review?

Additional Information

nithinraok commented Nov 21, 2023

github-actions bot commented Dec 30, 2023

titu1994 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pzelasko Jan 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pzelasko commented Jan 9, 2024

titu1994 left a comment

Choose a reason for hiding this comment

titu1994 commented Jan 9, 2024

pzelasko commented Jan 10, 2024

VahidooX left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pzelasko commented Jan 12, 2024

titu1994 left a comment

Choose a reason for hiding this comment

pzelasko commented Jan 13, 2024

pzelasko commented Nov 13, 2023 •

edited

Loading

pzelasko Jan 13, 2024 •

edited

Loading