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

[Recipe PR] MELD: Multimodal EmotionLines Dataset #4771

Merged
merged 22 commits into from Nov 28, 2022

Conversation

realzza
Copy link
Contributor

@realzza realzza commented Nov 17, 2022

About

Added baseline recipe for MELD: Multimodal EmotionLines Dataset

Checklist

  • (meld) be careful about the name for the recipe. It is recommended to follow naming conventions of the other recipes
  • common/shared files are linked with soft link (see Section 1.3.3)
  • modified or new python scripts should be passed through latest black formating (by using python package black). The command to be executed could be black espnet espnet2 test utils setup.py egs*/*/*/local egs2/TEMPLATE/*/pyscripts tools/*.py ci/*.py
  • modified or new python scripts should be passed through latest isort formating (by using python package isort). The command to be executed could be isort espnet espnet2 test utils setup.py egs*/*/*/local egs2/TEMPLATE/*/pyscripts tools/*.py ci/*.py
  • cluster settings should be set as default (e.g., cmd.sh conf/slurm.conf conf/queue.conf conf/pbs.conf)
  • update egs/README.md or egs2/README.md with corresponding recipes
  • add corresponding entry in egs2/TEMPLATE/db.sh for a new corpus
  • try to simplify the model configurations. We recommend to have only the best configuration for the start of a recipe. Please also follow the default rule defined in Section 1.3.3
  • large meta-information for a corpus should be maintained elsewhere other than in the recipe itself
  • recommend to also include results and pre-trained model with the recipe
    • results
    • (TODO) model

Todo

  • submit model through huggingface

@realzza realzza marked this pull request as ready for review November 17, 2022 08:31
@sw005320 sw005320 added this to the v.202211 milestone Nov 17, 2022
@sw005320
Copy link
Contributor

@siddhu001, it would be great if you also review this PR.

@sw005320 sw005320 added Recipe SLU Spoken language understanding labels Nov 17, 2022
Copy link
Collaborator

@ftshijt ftshijt left a comment

Choose a reason for hiding this comment

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

Just to confirm: it seems that this is only an unimodal model. Is it intentional or you will keep update it.

I also recommend you to check the task of SLU, which could be a better place to hold your recipe as well (we can keep on this).

egs2/meld/asr1/local/data_prep.py Outdated Show resolved Hide resolved
egs2/meld/asr1/local/data_prep.py Outdated Show resolved Hide resolved
egs2/meld/asr1/local/path.sh Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/run.sh Outdated Show resolved Hide resolved
egs2/meld/asr1/README.md Show resolved Hide resolved
@siddhu001
Copy link
Collaborator

I would suggest incorporating all of Jiatong's feedback. I added a comment and clarification, rest looks good to me.

@realzza
Copy link
Contributor Author

realzza commented Nov 18, 2022

Just to confirm: it seems that this is only an unimodal model. Is it intentional or you will keep update it.

Yes, this recipe is designed to use unimodal to do multitask learning is ASR and SLU. We may not update MELD to involve multimodals.

I also recommend you to check the task of SLU, which could be a better place to hold your recipe as well (we can keep on this).

Thanks for mentioning, I categorized this recipe as SLU in egs2/README.md.

@sw005320
Copy link
Contributor

@ftshijt, can you review it again?
I think this PR is almost ready.

@ftshijt
Copy link
Collaborator

ftshijt commented Nov 21, 2022

Looks cool! After it passed the CI tests, I will merge it

@realzza
Copy link
Contributor Author

realzza commented Nov 21, 2022

Seems it fails the CI test, as well as centos7 and debian9 tests due to the overlength lines in data_prep.py, let me fix this format issue shortly!

update: should be fixed now!

@realzza
Copy link
Contributor Author

realzza commented Nov 22, 2022

Hi @ftshijt, thank you for your review! I fixed the flake8 error W605 invalid escape sequence '\-', we can rerun the CI tests.

@codecov
Copy link

codecov bot commented Nov 22, 2022

Codecov Report

Merging #4771 (fbfe277) into master (ca2193d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4771   +/-   ##
=======================================
  Coverage   80.32%   80.32%           
=======================================
  Files         530      530           
  Lines       46527    46527           
=======================================
  Hits        37372    37372           
  Misses       9155     9155           
Flag Coverage Δ
test_integration_espnet1 66.37% <ø> (ø)
test_integration_espnet2 48.87% <ø> (ø)
test_python 68.62% <ø> (ø)
test_utils 23.30% <ø> (ø)

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

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@realzza
Copy link
Contributor Author

realzza commented Nov 28, 2022

The failed check may be caused by this issue reported in the latest version of numpy.

@sw005320 sw005320 merged commit 9a97143 into espnet:master Nov 28, 2022
@sw005320
Copy link
Contributor

Thanks, @realzza!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ESPnet2 README Recipe SLU Spoken language understanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants