Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Closes #448 | Add/Update Dataloader alorese #541
Closes #448 | Add/Update Dataloader alorese #541
Changes from 13 commits
c58865d
1c99845
aaba4fc
8fcffaf
7513b50
81fde25
8d7f639
069dcb5
1094c1c
6d227cd
b4a61c6
1bffd96
874f856
a413ea5
fa7f8b5
f22e93b
7bfca50
2fa1a1e
4876bf8
bef5cb2
0677796
118063a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may I ask few qns here?
overall, the schema we will use in here are T2T for translation of transcription of same audio from Indonesian to Alorese, and the SPText is the ASR version, right?
Which language does the audio content has? Indonesian or Alorese? bcs when I looked the code, there's no clear way to tell which lang is available in the transcription (and it's crucial to correctly map the Audio to the correct Transcriptions)
For the config name below:
{_DATASETNAME} _sptext_seacrowd_sptext
and{_DATASETNAME}_t2t_seacrowd_t2t
, may I know why the naming isn't something like{_DATASETNAME}_seacrowd_sptext
and{_DATASETNAME}_seacrowd_t2t
? any justifications here?I saw in the source, the info on
speaker_id
went missing? I thoughtsource
config should include all columns and informations (and stitch them appropriately if the data scattered into different files and configs, like what you did in your code)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sabilmakbar , thanks for the review and questions!
sptext
for alorese orsptext_trans
for indonesian. The mapping is done in this partBecause of the subset naming, the testing code (and dataset naming format) that was constructed needs to be in
<DATASET_NAME>_<SUBSET_NAME>_seacrowd_
`. It's just the subset name happens to be same as the schema, so it might be confusing. Happy to change if it might be needed.Thanks for the input! Please review the latest commit as I have done the nitpick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the prompt reply, @patrickamadeus!
Idk whether we need to create a schema of alorese audio against Indonesian text for the SPText schema. my personal opinion is to remove it since it could be very misleading (ASR schema should provide a text which is an actual transcription as is, not the translated ones)
If I understand this correctly, the subsets for this dataset are only the Alorese version and Indonesian version. The SPText and T2T don't fit properly to the definition of "subset" of a dataset, as only the schema is different.
wdyt? @ljvmiranda921 @holylovenia @SamuelCahyawijaya
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, hmm, does it mean we should have a separate data loaders for the SPText and the T2T versions? I don't have a particularly strong opinion on any approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we can have it in a single dataloader; I think the subset and configuration naming should be modified slightly.
The text information provided in this dataset is sequential; i.e., for every audio file, there is a sequence of annotated texts with their start & end timestamps.
For the Alorese language of text and audio, this can be put in ASR schema (or even a sequential audio split to its annotated text if we want to refine it further).
However, I don't think we should create an ASR schema for the Alorese audio and translated Indonesian annotation since the audio and text are in different languages.
And for T2T of Alorese and Indonesian translation, the existing implementation is correct, just need to reconstruct the configs list.
Therefore, my proposed configs are:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @sabilmakbar's suggestion.
Btw, just confirming, do the audio recordings and the transcriptions match word-for-word, @patrickamadeus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for the suggestions @sabilmakbar !
Yes! There are multiple timestamps to indicate when each word is spoken. @holylovenia , to be honest I haven't reviewed substantial sample from the data to determine whether it matched word-for-word or no, but as I listened to the first 10 seconds of 1 particular example, it perfectly matched.
If there is no further suggestion or comment, I will implement the change maximum this weekend, got a bunch of stuff to do first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sabilmakbar ! Could you please check on the latest commit? I have done the revision 👍.
For this one, I went on using the previous implementation first.