Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding MFA scripts #4557

Closed
wants to merge 11 commits into from
Closed

Adding MFA scripts #4557

wants to merge 11 commits into from

Conversation

iamanigeeit
Copy link
Contributor

@iamanigeeit iamanigeeit commented Aug 4, 2022

From #4521 : this commit adds scripts necessary to convert MFA to ESPnet formats.

  • mfa.sh automates the entire conversion process
  • tts.sh has been modified to pass arguments to data.sh for MFA
  • data.sh has been edited to produce required text and durations files

To create .lab files for other datasets, define the relevant function in mfa_format.py.

…The recipe `mfa.sh` details the entire conversion process, and `tts.sh` and `data.sh` (for LJSpeech only) have been edited to produce required text and durations files.
…The recipe `mfa.sh` details the entire conversion process, and `tts.sh` and `data.sh` (for LJSpeech only) have been edited to produce required text and durations files.
@mergify mergify bot added the ESPnet2 label Aug 4, 2022
@sw005320 sw005320 added this to the v.202209 milestone Aug 4, 2022
Copy link
Member

@kan-bayashi kan-bayashi left a comment

Choose a reason for hiding this comment

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

Sorry for the late reviewing and thank you for adding cool features!
Could you reflect my comments and fix the CI errors?

Comment on lines 309 to 310
local/data.sh ${local_data_opts}
local_data_opts+="--token_type ${token_type} --g2p ${g2p}"
local/data.sh "${local_data_opts}"
Copy link
Member

Choose a reason for hiding this comment

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

Please do not touch here, which may affect all of the recipes.
You can add local_data_opts in run.sh.
And ${local_data_opts} is different "${local_data_opts}" (latter is not parsed.)

Comment on lines 16 to 19
log "$0 $*"
token_type=''
g2p=''
. utils/parse_options.sh
Copy link
Member

Choose a reason for hiding this comment

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

How about adding use_mfa ?

Suggested change
log "$0 $*"
token_type=''
g2p=''
. utils/parse_options.sh
use_mfa=false
token_type=phn
g2p=g2p_en_no_space
log "$0 $*"
. utils/parse_options.sh
if "${use_mfa}"; then
if [ ${token_type} != "phn" ]; then
echo "ERROR: token_type must be phn when use_mfa=true."
exit 1
fi
if [ ${g2p} != "none" ]; then
echo "ERROR: g2p must be none when use_mfa=true."
exit 1
fi
fi

Copy link
Member

Choose a reason for hiding this comment

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

And could you add mfa installation check in the if expression?

<(cut -d "|" -f 1 < ${db_root}/LJSpeech-1.1/metadata.csv) \
<(cut -d "|" -f 3 < ${db_root}/LJSpeech-1.1/metadata.csv) \
> ${text}
if [ "${token_type}" = 'phn' ] && [ -z "${g2p}" ]; then # text should be phonemes!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [ "${token_type}" = 'phn' ] && [ -z "${g2p}" ]; then # text should be phonemes!
if "${use_mfa}"; then # text should be phonemes!

--n_shift "${n_shift}" \
--token_type phn \
--cleaner none \
--g2p none \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
--g2p none \
--g2p none \
--local_data_opts "--token_type phn --g2p none --use_mfa true" \

Comment on lines 1 to 3
#!/usr/bin/env bash
# Set bash to 'debug' mode, it will exit on :
# -e 'error', -u 'undefined variable', -o ... 'error in pipeline', -x 'print commands',
Copy link
Member

Choose a reason for hiding this comment

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

Please add the comments about this script especially focusing on difference of run.sh.

Copy link
Member

Choose a reason for hiding this comment

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

And if you make separated script, I think it is better to add all options in this scripts in order to train by just executing ./run_mfa.sh.

Comment on lines 1 to 8
set -e

# If you are not using LJSpeech, be sure to define how your dataset is processed in mfa_format.py
# Three

#!/usr/bin/env bash
# conda config --append channels conda-forge
# conda install montreal-forced-aligner
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
set -e
# If you are not using LJSpeech, be sure to define how your dataset is processed in mfa_format.py
# Three
#!/usr/bin/env bash
# conda config --append channels conda-forge
# conda install montreal-forced-aligner
#!/usr/bin/env bash
# Generate MFA alignement
# You need to install the following tools to run this script:
# $ conda config --append channels conda-forge
# $ conda install montreal-forced-aligner
# If you are not using LJSpeech, be sure to define how your
# dataset is processed in `scripts/utils/mfa_format.py`.
set -e

Comment on lines 48 to 68


# Run the below yourself

#./run_mfa.sh --stage 0 --stop_stage 0
#./run_mfa.sh --stage 1 --stop_stage 1
#./run_mfa.sh --stage 2 --stop_stage 2
#./run_mfa.sh --stage 3 --stop_stage 3
#./run_mfa.sh --stage 4 --stop_stage 4
#./run_mfa.sh --stage 5 --stop_stage 5 \
# --train_config conf/tuning/train_fastspeech2.yaml \
# --teacher_dumpdir data \
# --tts_stats_dir data/stats \
# --write_collected_feats true
#./run_mfa.sh --stage 6 --stop_stage 6 \
# --train_config conf/tuning/train_fastspeech2.yaml \
# --teacher_dumpdir data \
# --tts_stats_dir data/stats



Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Run the below yourself
#./run_mfa.sh --stage 0 --stop_stage 0
#./run_mfa.sh --stage 1 --stop_stage 1
#./run_mfa.sh --stage 2 --stop_stage 2
#./run_mfa.sh --stage 3 --stop_stage 3
#./run_mfa.sh --stage 4 --stop_stage 4
#./run_mfa.sh --stage 5 --stop_stage 5 \
# --train_config conf/tuning/train_fastspeech2.yaml \
# --teacher_dumpdir data \
# --tts_stats_dir data/stats \
# --write_collected_feats true
#./run_mfa.sh --stage 6 --stop_stage 6 \
# --train_config conf/tuning/train_fastspeech2.yaml \
# --teacher_dumpdir data \
# --tts_stats_dir data/stats
echo "Successfully finished generating MFA alignments.
# NOTE(iamanigeeit): If you want to train FastSpeech2 with the alignments,
# please check `egs2/ljspeech/tts1/run_mfa.sh` as an example.

Comment on lines 1 to 6
'''
This script converts files to/from MFA format for use in ESPnet TTS.
If you wish to add functions to create .lab files for MFA, add them like this:
`def make_labs_[dataset]:
...`
'''
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'''
This script converts files to/from MFA format for use in ESPnet TTS.
If you wish to add functions to create .lab files for MFA, add them like this:
`def make_labs_[dataset]:
...`
'''
#!/usr/bin/env python3
"""Convert files to/from MFA format for use in ESPnet TTS.
If you wish to add functions to create .lab files for MFA, add them like this:
def make_labs_[dataset]:
...
"""

@kan-bayashi kan-bayashi added New Features TTS Text-to-speech labels Aug 8, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 29, 2022

This pull request is now in conflict :(

@mergify mergify bot added conflicts ESPnet1 ASR Automatic speech recogntion MT Machine translation Installation labels Aug 29, 2022
@mergify mergify bot removed the conflicts label Sep 23, 2022
@mergify
Copy link
Contributor

mergify bot commented Sep 23, 2022

This pull request is now in conflict :(

@mergify mergify bot added the conflicts label Sep 23, 2022
@kan-bayashi kan-bayashi modified the milestones: v.202209, v.202211 Oct 4, 2022
@iamanigeeit
Copy link
Contributor Author

@kan-bayashi Sorry for the delay because of ICASSP submission. I will pull the latest version of ESPnet and fix my PR this week!

@mergify mergify bot added the README label Nov 14, 2022
� Conflicts:
�	README.md
�	espnet2/asr/encoder/wav2vec2_encoder.py
�	espnet2/asr/frontend/s3prl.py
�	espnet2/asr/postencoder/hugging_face_transformers_postencoder.py
�	espnet2/gan_tts/jets/jets.py
�	setup.py
�	test/espnet2/asr/frontend/test_s3prl.py
@mergify mergify bot removed the conflicts label Nov 29, 2022
@iamanigeeit
Copy link
Contributor Author

Hello @kan-bayashi

I have made the requested changes, but i can't submit the PR, because i have modified other files for my own work on iamanigeeit:master.

I tried to fork espnet again, but i am only allowed to fork once. When i create a new branch iamanigeeit:espnet/mfa and try to create a new PR from there, i get

Can't create a new pull request: Although you appear to have the correct authorization credentials, the espnet organization has enabled OAuth App access restrictions, meaning that data access to third-parties is limited.

What should i do?

@kan-bayashi
Copy link
Member

Thank you for your modification, @iamanigeeit.

Can't create a new pull request: Although you appear to have the correct authorization credentials, the espnet organization has enabled OAuth App access restrictions, meaning that data access to third-parties is limited.

I've never met this kind of error. Let me confirm your procedure.

  1. make new branch and perform cherry-pick in your local
  2. push your branch to your fork iamanigeeit/espnet.
  3. make PR from github HP

Is it correct?

@mergify
Copy link
Contributor

mergify bot commented Nov 30, 2022

This pull request is now in conflict :(

@mergify mergify bot added the conflicts label Nov 30, 2022
This was referenced Dec 2, 2022
@iamanigeeit
Copy link
Contributor Author

Thanks @kan-bayashi

Creating a new branch and making the PR from Github instead of Pycharm seems to work. The new commit is at #4801

@iamanigeeit iamanigeeit closed this Dec 2, 2022
@mergify mergify bot removed the conflicts label Dec 2, 2022
@Fhrozen Fhrozen mentioned this pull request Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASR Automatic speech recogntion ESPnet1 ESPnet2 Installation MT Machine translation New Features README TTS Text-to-speech
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants