search

mne-tools / mne-icalabel

Automatic labeling of ICA components in Python.
https://mne.tools/mne-icalabel/dev/index.html
BSD 3-Clause "New" or "Revised" License
94 stars 15 forks source link

Notes on JOSS paper #83

Closed TomDonoghue closed 2 years ago

TomDonoghue commented 2 years ago

Some brief comments / suggestions on the current draft of the JOSS paper:

Note: this issue is part of the JOSS review: https://github.com/openjournals/joss-reviews/issues/4484

adam2392 commented 2 years ago

Some brief comments / suggestions on the current draft of the JOSS paper:

  • lines 16-18: I find this sentence a little vague, and a couple sentences could clarify the details

    • how it makes this available: "this work makes the popular Matlab-based ICLabel model available in MNE-Python
    • something more on what "modern Pytorch format", as this is a bit unclear

Thanks for the comment, we've changed the sentence from: This work adds 17 the popular ICLabel model (Pion-Tonachini et al., 2019) to the MNE-Python (Gramfort et al., 18 2013) software toolkit in a modern Pytorch format (Paszke et al., 2019). to now say the following:

This work makes the popular ICLabel model [@iclabel2019] available in Python by creating a software package compatible with MNE-Python [@Agramfort2013] software toolkit and a modern Pytorch format [@Pytorch2019]. The ICLabel model was previously only available in an outdated version of tensorflow that was no longer supported, and migrating the model now to an updated Pytorch version will ensure the model will not break due to unmaintained versions of software.

  • line 41: say "we will build" but since this project now exists, the tense shouldn't be future

Here, we meant to specify our future plans for the repository, which would extend beyond just the ICLabel model. I have modified the sentence to state:

In future versions, we plan on building on top of the ICLabel model to improve its robustness when operating on different types of recording hardware, sensor type, and sensor count.

adam2392 commented 2 years ago
  • the relationship between Pytorch / Tensorflow, and what this project does related to them is unclear. For the M/EEG researcher who's looking into this project, I think it may be unclear how / why you jump between these different projects without detailing how they relate to each other and a more detailed description of the status of the old tool, and the update here.

I added the following sentences in the Statement of Need.

Pytorch is the most common Python deep-learning framework among researchers. Replicating the model in Pytorch enables future automatic ICA annotation research to easily build upon the model.

Does this address this concern?

jacobf18 commented 2 years ago

@adam2392 Small fix, but PyTorch is spelled with camel case.