Add SLUE_PERB codebase #5685
Add SLUE_PERB codebase #5685
Conversation
sw005320
added
SLU
egs2/slue-voxpopuli/asr1/local/data_prep_original_slue_format.py
Outdated
Show resolved
Hide resolved
| @@ -35,7 +35,7 @@ fi | |||
|
|
|||
| if [ ${stage} -le 1 ] && [ ${stop_stage} -ge 1 ]; then | |||
| if [ ! -e "${VOXPOPULI}/LICENSE.txt" ]; then | |||
| echo "stage 1: Download data to ${VOXPOPULI}" | |||
| echo "stage 1: Please download data to ${VOXPOPULI}" | |||
There was a problem hiding this comment.
It will be good to also print a link to dataset download instructions.
There was a problem hiding this comment.
Added in the README
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5685 +/- ##
===========================================
+ Coverage 32.51% 75.71% +43.20%
===========================================
Files 768 749 -19
Lines 70372 69738 -634
===========================================
+ Hits 22878 52802 +29924
+ Misses 47494 16936 -30558
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@pyf98, can you also review this PR? |
There was a problem hiding this comment.
Why is the filename capitalized?
There was a problem hiding this comment.
Hi I want to keep the same name as class name of s3prl function I copied: https://github.com/s3prl/s3prl/blob/66b54fd85980b9a822a7e5ce6f342b7d0edfcc23/s3prl/nn/upstream.py#L234. But I agree, I think it is better to keep it standard and not capitalized.
|
I have a high-level discussion about the desgin: |
|
Hi @pyf98, Thanks a lot for the suggestion! Yes, I had to modify the ASR files because, for summarization, we need to pre-train the model first on the ASR task and then fine-tune it on the summarization task. Although I made only a few changes to the ASR file, I agree with you that, in general, the ASR files are becoming complicated. In future, I will ensure to only manipulate the encoded features while fine-tuning the downstream task. However, for this PR, to ensure reproducibility with the results I obtained, I believe adding these changes to the ASR file is may be necessary. |
egs2/TEMPLATE/asr1/db.sh
Outdated
There was a problem hiding this comment.
Can you add some information about each corpus to https://github.com/espnet/espnet/blob/master/egs2/README.md?
egs2/slue-ted/slu1/README.md
Outdated
There was a problem hiding this comment.
I found that some configs have the corresponding models while others are not.
If you intentionally did it, that is fine.
If not, maybe we can upload a model (at least major ones, e.g., whisper and wavlm)
egs2/slue-voxpopuli/slu1/README.md
Outdated
There was a problem hiding this comment.
ditto.
Please deal with model upload appropriately.
| @@ -161,11 +162,22 @@ | |||
| type_check=AbsEncoder, | |||
| default="rnn", | |||
| ) | |||
| prepostencoder_choices = ClassChoices( | |||
There was a problem hiding this comment.
It would be better to explain what is prepostencoder in the source.
There was a problem hiding this comment.
Does it go through the test?
If not, please add it.
|
Hi @siddhu001, can you reflect on my comments? |
|
Yes I will reflect on your comments and update the PR soon. |
What?
Add required codebase for training SLUE-PERB models