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

sqaud dataset bug fixed #84

Closed

Conversation

Jonathan-Chin328
Copy link

Hi, I have fixed some bugs in generic_dataset.py and run it successfully

  1. The original BEGIN can't be detected from x in line 426
    (I printed x and found it is 'beginanswer' which without the space)
  2. A simple misnomer of variable

Also, I have some problems but don't know how to fix them properly yet

  1. Due to the different data format of prediction (compare to salesforce/decaNLP) ,
    it can't metric normally on task woz.en and wikisql

@s-jse
Copy link
Member

s-jse commented Feb 12, 2021

Thank you @Jonathan-Chin328 for this PR!

We don't usually use this codebase for datasets other than Almond (tasks/almond_dataset.py), so they can break easily when we are making changes.
If you can add a small test for SQuAD like our Almond dataset tests, we can make sure we keep the code compatible with it.

To add tests for a new dataset, you would need to:

  1. Add a very very small subset of that dataset in the tests/dataset folder
  2. Add a test command in tests/test.sh

Similarly, for datasets that don't currently work, if you add a test for them it will be much easier for us to fix and then maintain them.

@Jonathan-Chin328
Copy link
Author

Thanks for your response!
I have added the datasets of some tasks.

@Jonathan-Chin328
Copy link
Author

Jonathan-Chin328 commented Feb 25, 2021

I found that you have changed the output format of data preparing function few days ago(task.get_splits in train.py line 107),
which made all the others task can't work.
So I think I would just provide the datasets of the other tasks first~

@Mehrad0711
Copy link
Member

Mehrad0711 commented Nov 11, 2021

Hi @Jonathan-Chin328 ,
Was wondering if you're still interested in working on this?
Otherwise, I'll close this PR and submit an issue instead.

@Jonathan-Chin328
Copy link
Author

Jonathan-Chin328 commented Nov 19, 2021 via email

@Mehrad0711
Copy link
Member

Using #231 to track the issue.

@Mehrad0711 Mehrad0711 closed this Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants