search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: Off-resonance CorrecTion OPen soUrce Software (OCTOPUS) #2578

Closed whedon closed 3 years ago

whedon commented 4 years ago

Submitting author: @imr-framework (Marina Manso Jimeno) Repository: https://github.com/imr-framework/OCTOPUS Version: v1.0.0 Editor: @jni Reviewers: @puolival, @emilljungberg Archive: 10.5281/zenodo.4574444

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/4627d3e6f4f5ba10d9203c7574145e77"><img src="https://joss.theoj.org/papers/4627d3e6f4f5ba10d9203c7574145e77/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/4627d3e6f4f5ba10d9203c7574145e77/status.svg)](https://joss.theoj.org/papers/4627d3e6f4f5ba10d9203c7574145e77)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@puolival, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @jni know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @puolival

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @emilljungberg

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

Mmj94 commented 3 years ago

Hello all,

I'm back with the latest updates on the manuscript. I hope I have assessed all you comments @emilljungberg.

If you have done 3D stack of spirals, that's great and shows that it can be done. Any chance you can include an example of those results in the manuscript? So if this capability exists, then maybe you can reword it to something like "Correctio of 3D volumes is possible by processing individual 2D slices (if possible)"

Would you consider changing the sequence used for your simulation to one that would be strongly affected by off-resonance variations on the order of +/- 200 Hz or something like that?

Please let me know if you have further inputs/questions.

Sorry for all the PDFs generated above, please refer to the last one.

PS: I'm having some trouble including both my last names without using a hyphen. @jni any advice with this issue?

jni commented 3 years ago

PS: I'm having some trouble including both my last names without using a hyphen. @jni any advice with this issue?

Hmm, a very good question! I don't see how the current authorship format can disambiguate between first, middle, and last names. @openjournals/joss-editors any advice on this?

jni commented 3 years ago

I've been scouring the for yaml-style authorship definitions, and various pandoc doc pages, and I wonder whether LaTeX formatting will work here: - name: Manso Jimeno, Marina?

I think I've tracked down the bit of the whedon source code that does the name parsing, but my Ruby is not good enough to follow the parsing logic:

https://github.com/openjournals/whedon/blob/92346a0773a4003bf0ef361b661dc2787f492936/lib/whedon/author.rb#L2

Which appears to use this class:

https://github.com/chorn/nameable/blob/4dc46df3ee3080a054eb639839d39a66c8568259/lib/nameable/latin.rb#L5

It does not seem to me that the Lastname, Firstname pattern will work, but I wonder whether quotes will work somehow. I think I'll just ping @arfon for this one, since he wrote that author.rb code! 😬

arfon commented 3 years ago

Hmm, a very good question! I don't see how the current authorship format can disambiguate between first, middle, and last names. @openjournals/joss-editors any advice on this?

Unfortunately I will have to fix this in post-production (i.e., once the paper is already accepted and published). In the longer-term, we're planning on updating the metadata that JOSS allows for author names (to make it explicit what the last/family name is) but we can't do that yet sorry.

emilljungberg commented 3 years ago

Hi @Mmj94

Thanks for sending the revised version of the manuscript. I think these revisions looks really nice. The EPI and spiral readout off-resonance corrections looks great and do show the type of artefacts that are expected at the given off-resonance range. One question though, why did you change the B0 map model from the last version? The previous version had like a smooth almost like linear field map.

In the second part of the results where you do the noise comparison, you use the Cartesian acquisition which is no longer defined in the latest revision. Would you consider doing that analysis with the EPI or spiral results instead to make it consistent?

It is great to see the phantom results included in this version. On the top row, I'm not sure what improvement I'm expected to see, can you clarify? In the bottom row I can see that the three diagonal elements are better resolved after correction, would you consider making an arrow to them instead of the rectangle? It would help to both know what to focus on and also to see the edges of the black box which are currently covered by the rectangle.

Otherwise, I have no further comments. Looks great!

Thanks!

Mmj94 commented 3 years ago

Hi @emilljungberg,

I updated the manuscript with your latest comments:

  1. Modified the noise simulation experiment (Figure 2) from Cartesian to EPI trajectory to make it consistent.
  2. Improved regions on the images after correction (Figure 3) are now indicated with an arrow.

As per your questions:

why did you change the B0 map model from the last version?

I believe this field map is more "realistic" and resembles more a brain field map.

It is great to see the phantom results included in this version. On the top row, I'm not sure what improvement I'm expected to see, can you clarify?

Inside the inner circular structure of the phantom, one can appreciate smaller circles. I believe the edges of these circles are more discernable after off-resonance correction.

Thank you again!

Mmj94 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

emilljungberg commented 3 years ago

Hi @Mmj94

The PDF looks nice, I think I see the changes in the phantom data without the circles so that's greatly appreciated. I don't have any further comments on the manuscript then.

However, after these changes to the examples I downloaded the code again and tried to replicate the results from the paper but I'm afraid I'm not able to. When I run

python3 numsim_epi.py

It prints a field map that looks like this, which isn't the same as in the manuscript Fieldmap_EPI_75Hz

And this is the final result from the EPI simulation EPI_results

Is there a chance you didn't push the latest changes to the repository? I notice the the off resonance range is different and also the shape of the fieldmap as shown above.

Mmj94 commented 3 years ago

Hi @emilljungberg,

I thought about these example scripts as templates that the user could modify to experiment with the effect of different field maps, off-resonance ranges, k-space trajectories, etc. However, I just pushed the code so the default parameters can reproduce the figures on the paper.

Hope this helps!

emilljungberg commented 3 years ago

Hi @Mmj94

Thanks! I did notice now that the reference to running the scripts is not in the paper anymore which it was originally, I think that's why I was confused about not being able to reproduce the results. Having the example scripts reflect the figures in the paper is nice as it gives the user a direct check that they got it all working correctly.

I ran the numsim_epi.py and I get the same figures as you got so that is all fine.

With that, I don't have any more comments. Nice work!

emilljungberg commented 3 years ago

@jni I have now ticked all of my boxes in the review checklist and I'm happy with proceeding to acceptance of this work.

Mmj94 commented 3 years ago

Thank you all again, for your help and suggestions. I had fun and learned a lot during this review process :)

jni commented 3 years ago

@Mmj94 here we go!

Next steps: could you make a new tagged release of the software, archive it (e.g. with Zenodo), then report the version number and DOI here?

Mmj94 commented 3 years ago

Version number: v1.0.0 DOI: 10.5281/zenodo.4574444 https://zenodo.org/badge/latestdoi/214007114

jni commented 3 years ago

@whedon check references

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.2217/iim.10.33 is OK
- 10.1109/42.108599 is OK
- 10.1002/1522-2594(200102)45:2<269::AID-MRM1036>3.0.CO;2-5 is OK
- 10.1002/mrm.21599 is OK
- 10.1109/42.781014 is OK
- 10.1109/42.3926 is OK
- 10.1002/mrm.1910250210 is OK
- 10.1002/mrm.1910370523 is OK
- 10.1002/mrm.22428 is OK
- 10.1016/j.mri.2017.07.004 is OK
- 10.1016/j.neuroimage.2011.09.015 is OK
- 10.1109/TMI.2002.808360 is OK
- 10.1109/TSP.2005.853152 is OK
- 10.1016/j.mri.2018.03.008 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1038/s41592-019-0686-2 is OK
- 10.5281/zenodo.3701467 is OK
- 10.1109/MCSE.2007.55 is OK
- 10.5281/zenodo.3891702 is OK
- 10.7717/peerj.453 is OK
- 10.1109/tns.1974.6499235 is OK

MISSING DOIs

- None

INVALID DOIs

- None
jni commented 3 years ago

@whedon set 10.5281/zenodo.4574444 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4574444 is the archive.

jni commented 3 years ago

@whedon set v1.0.0 as version

whedon commented 3 years ago

OK. v1.0.0 is the version.

jni commented 3 years ago

@whedon accept

whedon commented 3 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2131

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/2131, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
jni commented 3 years ago

@arfon as a reminder, we need to remove the hyphen from Marina's last name in the final pdf and paper metadata. :pray:

Thank you so much everyone for your patience with this review, thank you @puolival and @emilljungberg for reviewing, and thank you @Mmj94 for all the iteration! :tada:

danielskatz commented 3 years ago

@Mmj94 - please see the changes suggested in https://github.com/imr-framework/OCTOPUS/pull/8 and merge them, or let me know what you disagree with, then we can proceed to final acceptance and publication

Mmj94 commented 3 years ago

@danielskatz done

danielskatz commented 3 years ago

👋 @arfon - given the specific handling needed, can you do the final accept here? (rather than me doing it and then you fixing it afterwards)

arfon commented 3 years ago

👋 @arfon - given the specific handling needed, can you do the final accept here? (rather than me doing it and then you fixing it afterwards)

Certainly. I've gone ahead and processed this manually.

@puolival, @emilljungberg - many thanks for your reviews here and to @jni for editing this submission! JOSS relies upon the volunteer efforts of people like you, and we simply wouldn't be able to do this without you ✨

@Mmj94 - your paper is now accepted and published in JOSS :zap::rocket::boom:

whedon commented 3 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.02578/status.svg)](https://doi.org/10.21105/joss.02578)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.02578">
  <img src="https://joss.theoj.org/papers/10.21105/joss.02578/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.02578/status.svg
   :target: https://doi.org/10.21105/joss.02578

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following: