[espnet3-14.3] Add integration test for ASR by Masao-Someki · Pull Request #6392 · espnet/espnet

Masao-Someki · 2026-03-17T04:41:37Z

What did you change?

Added ASR integration test

Why did you make this change?

To test espnet3 pipeline

Is your PR small enough?

No, 28 files change and 2,069 addition.

Except for the following edits, the reviews are done in #6331.

Please also add comments to the config like the that in the template directory

Additional Context

gemini-code-assist

Code Review

This pull request adds integration tests for ASR in espnet3, introducing a new recipe structure with template configurations and a more robust optimization and inference pipeline. The changes are extensive and well-structured, improving flexibility and maintainability. A key improvement is the refactoring of the optimizer and scheduler handling in the LightningModule, moving to a manual optimization loop for multi-optimizer setups which is cleaner and more aligned with PyTorch Lightning's practices. The inference pipeline is also enhanced with better artifact handling. However, I found a significant performance issue in the new CI script.

gemini-code-assist · 2026-03-17T04:45:19Z

+        --training_config conf/training.yaml \
+        --inference_config "${inference_config}" \
+        --metrics_config conf/metrics.yaml
+    rm -rf exp data


The rm -rf exp data command inside the run_with_training_config function, which is called in a loop, will cause the dataset to be downloaded and prepared repeatedly for each training configuration. This is highly inefficient and will significantly slow down the CI process. The create_dataset and train_tokenizer stages are idempotent and can be run just once before the loop. To fix this, you should remove data from this rm command to persist the prepared data across test runs within the same CI job. The exp directory should still be cleaned up to ensure a fresh state for each training run.

Suggested change

rm -rf exp data

rm -rf exp

codecov · 2026-03-17T05:09:18Z

Codecov Report

❌ Patch coverage is 93.73041% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.25%. Comparing base (48cb257) to head (8f24650).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
espnet3/systems/asr/transducer_task.py	91.86%	10 Missing ⚠️
espnet3/utils/run_utils.py	88.00%	6 Missing ⚠️
espnet3/components/data/dataset_module.py	94.44%	3 Missing ⚠️
espnet3/systems/asr/system.py	91.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6392      +/-   ##
==========================================
+ Coverage   70.16%   70.25%   +0.08%     
==========================================
  Files         787      791       +4     
  Lines       73367    73606     +239     
==========================================
+ Hits        51477    51710     +233     
- Misses      21890    21896       +6

Flag	Coverage Δ
test_integration_espnet2	`46.78% <ø> (ø)`
test_python_espnet2	`61.02% <0.00%> (-0.20%)`	⬇️
test_python_espnet3	`18.08% <93.73%> (+0.63%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…espnet into espnet3/integration_test_3

sw005320

There are many dataset-dependent names or paths in the config file.
Please avoid them as much as possible.
You should make them under the recipe directory and avoid specifying the dataset names.

I think the tokenizer part is well-designed.
Please follow this way

sw005320 · 2026-03-24T10:36:49Z

+exp_tag: ${load_yaml:training.yaml,exp_tag}
+inference_dir: ${exp_dir}/inference
+dataset_dir: ${data_dir}/mini_an4


This would be an issue as we need to specify the dataset name mini_an4, and it would cause some errors.
How about creating the dataset_dir under egs3/mini_anr so that we don't need to specify it?

Is there another way to avoid it?

sw005320 · 2026-03-24T10:40:07Z

+create_dataset:
+  func: src.creating_dataset.create_dataset
+  dataset_dir: ${dataset_dir}
+  archive_path: ${recipe_dir}/../../egs2/mini_an4/asr1/downloads.tar.gz


Ditto
Instead of specifying the data in egs2, it is better to put it under the recipe directory.

Also, please avoid using egs2 files.
Make espnet3 recipes independent as much as possible.

sw005320 · 2026-03-24T10:40:17Z

+task: espnet3.systems.asr.task.ASRTask
+
+exp_tag: train_asr_rnn_debug
+dataset_dir: ${data_dir}/mini_an4


sw005320 · 2026-03-24T10:41:10Z

+##########################################################
+task: espnet3.systems.asr.task.ASRTask
+
+exp_tag: train_asr_rnn_debug


Why _debug?
Please remove unnecessary names and avoid confusions

sw005320 · 2026-03-24T10:57:44Z

+  train:
+    - name: train_nodev
+      dataset:
+        _target_: src.dataset.MiniAN4Dataset


ditto
This would bring some issues as we need to specify the dataset name

sw005320 · 2026-03-24T11:10:34Z

+create_dataset:
+  func: src.creating_dataset.create_dataset
+  dataset_dir: ${dataset_dir}
+  archive_path: ${recipe_dir}/../../egs2/mini_an4/asr1/downloads.tar.gz


sw005320 · 2026-03-24T11:10:40Z

+  train:
+    - name: train_nodev
+      dataset:
+        _target_: src.dataset.MiniAN4Dataset


sw005320 · 2026-03-24T11:10:44Z

+  valid:
+    - name: train_dev
+      dataset:
+        _target_: src.dataset.MiniAN4Dataset


sw005320 · 2026-03-24T11:12:55Z

+    dataset_dir = Path(dataset_dir)
+    logger = setup_logger(name="mini_an4.create_dataset")
+
+    archive = Path("../../../egs2/mini_an4/asr1/downloads.tar.gz")


avoid to specify the espnet2 directory

sw005320 · 2026-03-24T11:15:44Z

This seems to be dataset independent.
It would be better to move this to the recipe common place (e.g., template)

…3/integration_test_3

…espnet into espnet3/integration_test_3

for more information, see https://pre-commit.ci

…espnet into espnet3/integration_test_3

sw005320 · 2026-03-30T19:40:17Z

Can you even remove mini_an4/asr from the config?
Since it will be under the mini_an4/asr recipe directory, we can extract this path information from the current directory information.

Masao-Someki · 2026-04-02T01:39:30Z

The change in c78db55 removes the load_yaml resolver and stops config files from reaching into sibling configs to fetch exp_tag / exp_dir.

Instead, experiment identity is handled at runner setup time:

configs are loaded with resolve=False
when training_config is present, exp_tag and exp_dir are propagated into inference_config and metrics_config
when training_config is absent, standalone inference/metrics configs must define exp_tag or a concrete exp_dir, otherwise the runner raises an error

Motivation

load_yaml looked convenient for reusing values across config files, but it created a weaker config-loading path.
In practice, that caused a few problems:

it read raw YAML files, so some configs defined by TEMPLATE/**/*.yaml or with defaults:, are not visible from the load_yaml resolver.
behavior became hard to track once configs started depending on other configs during interpolation

In the CI issue, the transducer failure was a concrete example of this..
inference_transducer.yaml read exp_dir from training.yaml, but load_yaml evaluated the raw training config without merging the default config, so resolution broke.
Rather than making cross-config reads more complex, this change removes that pattern and keeps experiment identity handling in run.py

What changed

removed load_yaml from espnet3/utils/config_utils.py
added runner-side experiment-context helpers in espnet3/utils/run_utils.py
updated egs3/TEMPLATE/asr/run.py to apply and validate experiment context before final resolve
removed load_yaml usage from mini_an4 inference / transducer inference / metrics configs
moved helper coverage into test/espnet3/utils/test_run_utils.py
updated config-related tests accordingly

Masao-Someki · 2026-04-06T19:13:10Z

@sw005320 If this looks okay to you, could you merge this PR? We can move to the next (3.14.5) bug fix PR!

…3/integration_test_3

…espnet into espnet3/integration_test_3

for more information, see https://pre-commit.ci

…espnet into espnet3/integration_test_3

Masao-Someki · 2026-04-08T01:43:42Z

I have fixed the config name. I will handle other config-related items in the following PRs!

sw005320 · 2026-04-08T11:46:06Z

+        python-version: ["3.10", "3.12"]
+        pytorch-version: [2.9.1, 2.10.0, 2.11.0]
+        use-conda: [false]
+        chainer-version: [6.0.0]


Do you need Chainer?

Thanks for the comment!
chainer-version was not actually used in the espnet3's CI workflow, so I removed it in 2e58522

sw005320 · 2026-04-08T11:50:39Z

+dataset:
+  test:
+    - name: test
+      data_src_args:


Do you need args?
data_src or data_sources would be good enough?

Also, it is better to add similar explanation comments to the training data

##########################################################

DATASET DEFINITION

##########################################################

Dataset splits:

train:

- split="train" from the dataset.dataset.Minian4Dataset

valid:

- split="valid" from the dataset.dataset.Minian4Dataset

Notes:

- actual data sources are defined via ${recipe_dir}

Thank you, and yes, I think it is good to have data_src_args.
This would be passed directly to the dataset class. For example, suppose we have a dataset class like:

class MyDataset(Dataset): def __init__(self, split="train",): self.data = self._build_data(split) ...

Then we can feed the data_src_args.split to the __init__().
We could remove data_src_args, but then we would need additional code to manually remove (pop) data_src from the config, and then feed the rest configs to __init__().

for more information, see https://pre-commit.ci

sw005320 · 2026-04-10T11:18:41Z

Please check my other comments

…espnet into espnet3/integration_test_3

Masao-Someki · 2026-04-11T03:41:49Z

Thank you!

Chainer is removed for espnet3's CI in 2e58522
I think data_src_args should stay as-is..
I added dataset explanation comments to the training config in 1bbaeb9 and c8a80bf!

sw005320 · 2026-04-08T11:51:02Z

+#     - uses the "test" split defined in the dataset.dataset.Minian4Dataset
+dataset:
+  test:
+    - name: test


I believe this should also include valid

@Masao-Someki, please check this comment
We usually do not have only the test data.
If it is intentional to reduce the computation cost, that is valid (but I think we do not have so many additions for the computation time)

sw005320 · 2026-04-08T11:52:33Z

+##########################################################
+#                    PATHS AND INPUTS                    #
+##########################################################
+inference_dir: ${exp_dir}/inference


Where do we specify the data?
Is it included in this directory specification?
I think it should be commented.

sw005320 · 2026-04-08T11:53:17Z

We can skip RNN related integration test.
This is not used anymore.

sw005320 · 2026-04-08T11:53:28Z

ditto
We can skip RNN related integration test.
This is not used anymore.

sw005320 · 2026-04-08T11:54:47Z

In this case, the data_aug part can be moved to the other config

sw005320 · 2026-04-08T11:56:34Z

+#     - name: librispeech_test_clean
+#       data_src: librispeech/asr
+#       data_src_args:
+#         split: test_clean
+#         data_path: ${dataset_dir}
+#     - name: librispeech_test_other
+#       data_src: librispeech/asr
+#       data_src_args:
+#         split: test_other
+#         data_path: ${dataset_dir}


The dev sets should be included

@Masao-Someki, please check this comment
We usually do not have only the test data.
If it is intentional to reduce the computation cost, that is valid (but I think we do not have so many additions for the computation time)

sw005320 · 2026-04-08T11:58:29Z

+        - data_src: mini_an4/asr
+          data_src_args:


These lines look redundant.
Do we really need data_src_args:?

Masao-Someki added 3 commits March 16, 2026 23:53

Add mini-an4 recipe

b29a31c

Add transducer task

19867aa

Fixed naming and added integration test script

c014a6d

dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. ASR Automatic speech recogntion CI Travis, Circle CI, etc ESPnet3 labels Mar 17, 2026

gemini-code-assist Bot reviewed Mar 17, 2026

View reviewed changes

Fhrozen added this to the v.202604 milestone Mar 17, 2026

Masao-Someki and others added 2 commits March 24, 2026 01:29

Merge branch 'espnet3/integration_test_2' of github.com:Masao-Someki/…

bf5f66e

…espnet into espnet3/integration_test_3

Merge branch 'master' into espnet3/integration_test_3

b0e0060

sw005320 reviewed Mar 24, 2026

View reviewed changes

Masao-Someki and others added 16 commits March 26, 2026 01:19

Merge branch 'master' of https://github.com/espnet/espnet into espnet…

56e99be

…3/integration_test_3

Moved dataset related scripts to dataset/ directory

25c81e1

Applied format

2484974

Fixed config

1f83fb5

Fixed dataset directory and configs

f2cbc6f

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

b96da67

…espnet into espnet3/integration_test_3

[pre-commit.ci] auto fixes from pre-commit.com hooks

497a056

for more information, see https://pre-commit.ci

Merge branch 'master' into espnet3/integration_test_3

a62df62

Moved archive data from egs2/ to egs3/

03cf93b

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

6e06418

…espnet into espnet3/integration_test_3

Added integration test for CI

2ca6ce1

chmod +x ci/test_integration_espnet3.sh

130bd3b

Removed debug comment out

bcbed9d

Merge branch 'master' into espnet3/integration_test_3

b10cc42

Fix bug in config

5cf5fdf

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

f51788a

…espnet into espnet3/integration_test_3

Masao-Someki added 2 commits April 4, 2026 15:47

Merge branch 'master' into espnet3/integration_test_3

2b2e13a

Merge branch 'master' into espnet3/integration_test_3

4b919ba

Fhrozen modified the milestones: v.202604, v.202607 Apr 7, 2026

Masao-Someki and others added 7 commits April 7, 2026 19:14

Replaced kwargs -> data_src

fa06530

Merge branch 'master' of https://github.com/espnet/espnet into espnet…

07bacb3

…3/integration_test_3

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

d7ce67f

…espnet into espnet3/integration_test_3

[pre-commit.ci] auto fixes from pre-commit.com hooks

83c3ded

for more information, see https://pre-commit.ci

Fixed pytorch version on CI

6de28e4

Fixed docstrings

e392908

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

2c7172d

…espnet into espnet3/integration_test_3

sw005320 reviewed Apr 8, 2026

View reviewed changes

Masao-Someki and others added 5 commits April 9, 2026 19:08

Removed chainer

2e58522

Add details on config

1bbaeb9

Applied format

7d17ff2

Merge branch 'master' into espnet3/integration_test_3

c339d32

[pre-commit.ci] auto fixes from pre-commit.com hooks

1b98c3f

for more information, see https://pre-commit.ci

Masao-Someki mentioned this pull request Apr 10, 2026

[espnet3.14.4] Use config file names as default experiment and inference directories #6416

Open

Masao-Someki added 2 commits April 10, 2026 23:39

Fixed dataset comment based on shinji's review

c8a80bf

Merge branch 'espnet3/integration_test_3' of github.com:Masao-Someki/…

263c0b1

…espnet into espnet3/integration_test_3

Removed unintentionally included test file

8f24650

dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Apr 14, 2026

sw005320 reviewed Apr 15, 2026

View reviewed changes

Conversation

Masao-Someki commented Mar 17, 2026

What did you change?

Why did you make this change?

Is your PR small enough?

Additional Context

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sw005320 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sw005320 commented Mar 30, 2026

Uh oh!

Masao-Someki commented Apr 2, 2026

Motivation

What changed

Uh oh!

Masao-Someki commented Apr 6, 2026

Uh oh!

Masao-Someki commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

DATASET DEFINITION

Dataset splits:

train:

- split="train" from the dataset.dataset.Minian4Dataset

valid:

- split="valid" from the dataset.dataset.Minian4Dataset

Notes:

- actual data sources are defined via ${recipe_dir}

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sw005320 commented Apr 10, 2026

Uh oh!

Masao-Someki commented Apr 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov Bot commented Mar 17, 2026 •

edited

Loading