adding NMT dataset

[javadbayazi]

As the NMT dataset is similar to TUH abnormal, with a few changes we can add this dataset to Braindecode!
Reference paper

[codecov]

Codecov Report

Merging #443 (b505acd) into master (c8d7de6) will increase coverage by 0.10%.
The diff coverage is 91.46%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #443      +/-   ##
==========================================
+ Coverage   84.00%   84.11%   +0.10%     
==========================================
  Files          66       67       +1     
  Lines        5338     5419      +81     
==========================================
+ Hits         4484     4558      +74     
- Misses        854      861       +7     

[robintibor]

Thanks for starting this!
Having NMT be a subclass of TUH seems a potentially confusing decision to me.
We also had some code here to load NMT: https://gist.github.com/gemeinl/e932169ed2eca5bf8b25432f0d54b095
Is there any difference in functionality between these two versions as far as you can see?

In order to avoid potential confusions and also that code may break for one dataset if the other is changed, I suggest we either keep them separate as in gist above or we define common functions that both use in a separate file. This might be overkill, definitely I think subclassing TUH here seems too high a risk of creating unintentional errors later

[javadbayazi]

Thanks for starting this! Having NMT be a subclass of TUH seems a potentially confusing decision to me. We also had some code here to load NMT: https://gist.github.com/gemeinl/e932169ed2eca5bf8b25432f0d54b095 Is there any difference in functionality between these two versions as far as you can see?

No, they are the same! I haven’t seen that gist and it worked for me with a few changes!

In order to avoid potential confusions and also that code may break for one dataset if the other is changed, I suggest we either keep them separate as in gist above or we define common functions that both use in a separate file. This might be overkill, definitely I think subclassing TUH here seems too high a risk of creating unintentional errors later

Sure! Having a separate file would be a better option.

[robintibor]

Thanks for starting this! Having NMT be a subclass of TUH seems a potentially confusing decision to me. We also had some code here to load NMT: https://gist.github.com/gemeinl/e932169ed2eca5bf8b25432f0d54b095 Is there any difference in functionality between these two versions as far as you can see?

No, they are the same! I haven’t seen that gist and it worked for me with a few changes!

In order to avoid potential confusions and also that code may break for one dataset if the other is changed, I suggest we either keep them separate as in gist above or we define common functions that both use in a separate file. This might be overkill, definitely I think subclassing TUH here seems too high a risk of creating unintentional errors later

Sure! Having a separate file would be a better option.

Great would you mind having a try to create this separate file then based on the gist?

[bruAristimunha]

Hi @MohammadJavadD,

do you need any help?

[javadbayazi]

@bruAristimunha I made the discussed change to create a separate class for the NMT dataset, Please take a look.

[bruAristimunha]

We need a fetch for the dataset. I am doing something @MohammadJavadD

[bruAristimunha]

Can you make the tests in the meantime, @MohammadJavadD?

There are plenty of tests for Temple University on braindecode, and you can follow the same type of mock test.

[javadbayazi]

We need a fetch for the dataset. I am doing something @MohammadJavadD

This might help

[bruAristimunha]

Thank you for the suggestion @MohammadJavadD, I saw your code, but I will use mne to fetch the data. I am uploading the dataset in a better place than google drive.

[bruAristimunha]

If you need any help, please let me know, on the test side.

[javadbayazi]

@bruAristimunha I added a mock dataset for the NMT and created some unit tests based on that.

[bruAristimunha]

Hey @apmellot, @jpaillard and @dengemann!

FYI.

We are adding this potential brain-age dataset, smaller than the Temple University Dataset, as the community uses the dataset, cc. @MohammadJavadD.

You may have options about the dataset, but as you work with the TUH dataset, I think you would like to know about it. If you have time to review, that would be nice, but okay otherwise.

[javadbayazi]

Hey @apmellot, @jpaillard and @dengemann!

FYI.

We are adding this potential brain-age dataset, smaller than the Temple University Dataset, as the community uses the dataset, cc. @MohammadJavadD.

You may have options about the dataset, but as you work with the TUH dataset, I think you would like to know about it. If you have time to review, that would be nice, but okay otherwise.

We tried sex prediction on this dataset here, I expect it should work for age as well, but I'm curious to know more about it.

[bruAristimunha]

Hey @MohammadJavadD,

Fetch working!

Can you upload your branch and test it to see if the dataset download is working? Too big to be tested in the CI.

[bruAristimunha]

I also think that one of your tests is not working.

[bruAristimunha]

Thank you so much @MohammadJavadD!

I am super happy with your contribution, and let's continue on the transfer learning tutorial ;)

[bruAristimunha]

and btw, thank you @neerajwagh for your google drive!

[javadbayazi]

Thank you @bruAristimunha for all your help.

[neerajwagh]

and btw, thank you @neerajwagh for your google drive!

No problem! It's worth checking with the NMT authors to see why the link was taken down or not maintained. They could be working on a newer version and can notify you of updates.

[bruAristimunha]

Some people have already tried, but no response unfortunately. It appears that the student has graduated, but not sure. Anyway, I think it is solved now.

…

On Thu, 4 Apr 2024, 22:17 Neeraj Wagh, ***@***.***> wrote: and btw, thank you @neerajwagh <https://github.com/neerajwagh> for your google drive! No problem! It's worth checking with the NMT authors to see why the link was taken down or not maintained. They could be working on a newer version and can notify you of updates. — Reply to this email directly, view it on GitHub <#443 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKFZNAQWJFEXVUIACH6XC5TY3WYPBAVCNFSM6AAAAAAWCYTAZGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZYGEZTENZRGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>