search

openjournals / joss-reviews

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

[REVIEW]: CLAIRE: Constrained Large Deformation Diffeomorphic Image Registration on Parallel Computing Architectures #3038

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @andreasmang (Andreas Mang) Repository: https://github.com/andreasmang/claire Version: v0.08 Editor: @mjsottile Reviewers: @tjasaki Archive: 10.5281/zenodo.4837427

: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/d4af0a15946fd2161935be018820243b"><img src="https://joss.theoj.org/papers/d4af0a15946fd2161935be018820243b/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/d4af0a15946fd2161935be018820243b/status.svg)](https://joss.theoj.org/papers/d4af0a15946fd2161935be018820243b)

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

@tjasaki & @NMontanaBrown, 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 @mjsottile 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 @tjasaki

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @NMontanaBrown

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @tjasaki, @NMontanaBrown it looks like you're currently assigned to review this paper :tada:.

: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.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.79 s (171.5 files/s, 68268.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C++                             44           8821           6972          27289
C/C++ Header                    48           1464           1122           3491
Bourne Shell                    14            330            317           1504
CMake                           14            165            418            989
Markdown                         9            179              0            480
TeX                              1             33              4            235
make                             3             40              7            180
Python                           1             17              7             60
YAML                             2              0              0              5
-------------------------------------------------------------------------------
SUM:                           136          11049           8847          34233
-------------------------------------------------------------------------------

Statistical information for the repository '181063aa74ae26a70dd5afc3' was
gathered on 2021/02/13.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Amir Gholami                     1             1              0            0.00
Andreas Mang                   588         94954          44608           91.03
James Herring                    5           765             91            0.56
MalteBrunn                       2           228           6005            4.07
Naveen Himthani                 21          4087           1328            3.53
brunnme                          1             6              0            0.00
naveenaero                       4          1241              6            0.81

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Amir Gholami                  1          100.0         33.8                0.00
Andreas Mang              46628           49.1         38.5               22.07
James Herring               702           91.8         20.0               16.10
MalteBrunn                  234          102.6         16.0                2.14
Naveen Himthani              13            0.3         14.5               15.38
naveenaero                 1673          134.8          9.6                6.81
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:

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

OK DOIs

- None

MISSING DOIs

- 10.1038/nprot.2014.123 may be a valid DOI for title: Advanced CLARITY for rapid and high-resolution imaging of intact tissues
- 10.1023/b:visi.0000043755.93987.aa may be a valid DOI for title: Computing large deformation metric mappings via geodesic flows of diffeomorphisms
- 10.1146/annurev-bioeng-062117-121105 may be a valid DOI for title: Integrated Biophysical Modeling and Image Analysis: Application to Neuro-Oncology
- 10.1007/978-3-030-59713-9_53 may be a valid DOI for title: Multiatlas calibration of biophysical brain tumor growth models with mass effect
- 10.1137/19m1275280 may be a valid DOI for title: Image-driven biophysical tumor growth model calibration
- 10.1109/tmi.2020.3024264 may be a valid DOI for title: Fully Automatic Calibration of Tumor-Growth Models Using a Single mpMRI Scan
- 10.1007/s11081-018-9390-9 may be a valid DOI for title: PDE-constrained optimization in medical image analysis
- 10.1137/18m1207818 may be a valid DOI for title: CLAIRE: A distributed-memory solver for constrained large deformation diffeomorphic image registration
- 10.1145/3126908.3126930 may be a valid DOI for title: A Framework for Scalable Biophysics-based Image Analysis
- 10.1137/140984002 may be a valid DOI for title: An inexact Newton–Krylov algorithm for constrained diffeomorphic image registration
- 10.1137/16m1070475 may be a valid DOI for title: A Semi-Lagrangian two-level preconditioned Newton–Krylov solver for constrained diffeomorphic image registration
- 10.1109/sc.2016.71 may be a valid DOI for title: Distributed-memory large-deformation diffeomorphic 3D image registration

INVALID DOIs

- None
mjsottile commented 3 years ago

Hi @andreasmang - while the reviewers start working through their checklists, can you address the missing DOIs in the references?

andreasmang commented 3 years ago

Hi @andreasmang - while the reviewers start working through their checklists, can you address the missing DOIs in the references?

@mjsottile: Thanks. I have added DOIs to all articles for which they exist to the bib file. Changes have been pushed to the repo. Let me know if there are any issues...

NMontanaBrown commented 3 years ago

Hi team, I've started review with two specific issues.

Unit tests Contributing Guidelines

whedon commented 3 years ago

:wave: @NMontanaBrown, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @tjasaki, please update us on how your review is going (this is an automated reminder).

mjsottile commented 3 years ago

:wave: @NMontanaBrown, @tjasaki Let me know if you need any assistance in completing the reviews. I see that @NMontanaBrown has created some issues in the project that the authors still need to respond to, so I assume that part of the review is ongoing.

NMontanaBrown commented 3 years ago

Hi @mjsottile I am waiting for unit tests on the code. I have more questions for the authors but I will wait for this to be addressed before proceeding further.

tjasaki commented 3 years ago

Hi @mjsottile I am in the midst of testing the installation. I am working through missing dependencies and broken links trying to determine the particular causes of the issues.

andreasmang commented 3 years ago

@tjasaki can you be more specific? There should not be any missing dependencies. Also, which branch (GPU or CPU)?

Considering the other comments. I will post a response soon. I am drowning the last couple of days and I was assuming I get feedback that we address at once as opposed to doing this in an iterative way.

tjasaki commented 3 years ago

Yes @andreasmang. I did not mean to imply that the fault could be all at your end. For example, I needed to install cmake on my system. Later today I can pass along more particulars about how the build went. We'll get there. Tom

andreasmang commented 3 years ago

@tjasaki Sorry. I also did not want to imply anything. Feedback is very much appreciated, especially when it comes to compilation related issues. If you want me to update the documentation considering particular issues, do not hesitate. We have several folks involved building this on different systems but they are mostly local (as in I work with them directly) and it's quite complicated due to the different dependencies and the fact that it is developed for HPC architectures...Plan is to increase the visibility and get more folks to use the software. So, again, I am grateful for any hints that help us iron out issues.

andreasmang commented 3 years ago

Also, we are currently pushing everything to the GPU. There are less dependencies on that branch, since the FFT kernels are now part of CLAIRE...This will become the default branch soon...

andreasmang commented 3 years ago

Dear all:

I apologize for the silence. The last couple of weeks have been pretty intense. I will be able to respond more quickly to further requests from now on.

Comment R1: Hi @andreasmang, how you have tested/validated your code and its outputs? Currently, as far as I can find, there is no automated test suite via Github Actions or other.

We did not discuss this in the README. We currently do not have support for Github Actions. This remains subject to future work.

CLAIRE features a test binary and some internal tests used in our group to make sure the computational kernels produce accurate results. We have added a description of these to the README file: https://github.com/andreasmang/claire/blob/gpu/doc/README-RUNME.md#testing

We also provide a brief description of how to use CLAIRE and include a link to data we have extensively considered in our prior work:

https://github.com/andreasmang/nirep

I am not going to mention additional details here but rather refer to the "overhauled" README environment.

Comment R2: Hi @andreasmang, what are the contributing guidelines for this project? They are missing from the repository.

Thanks for pointing this out. These did not exist. We have added contributing guidelines to the repository: https://github.com/andreasmang/claire/blob/master/doc/CONTRIBUTING.md

We have also included a "Code of Conduct" https://github.com/andreasmang/claire/blob/gpu/doc/CODE_OF_CONDUCT.md

Based on a prior comment posted above we have also modified the structure of the README.

We have also enabled "GitHub Discussions" to allow the community to make comments and suggestions.

Cheers A

mjsottile commented 3 years ago

:wave: @tjasaki and @NMontanaBrown : it looks like the authors have been making progress in responding to review topics. Let me know if you need anything to make progress on the reviews.

tjasaki commented 3 years ago

@mjsottile : I've been reading and reviewing the new material. I am still stymied by getting claire to install properly. I've made progress, it seems to be down to a CUDA (compatibility?) issue which I haven't been able to sort out yet. The installation instructions seem reasonable and have been notably helpful, but accounting for all possibilities would be unrealistic.

andreasmang commented 3 years ago

@mjsottile : I've been reading and reviewing the new material. I am still stymied by getting claire to install properly. I've made progress, it seems to be down to a CUDA (compatibility?) issue which I haven't been able to sort out yet. The installation instructions seem reasonable and have been notably helpful, but accounting for all possibilities would be unrealistic.

@tjasaki (and others): Let me know if there is anything we should do on our end...(i.e., if you would like us to check if we can emulate a particular setting and if we can figure it out). Feel free to raise an ISSUE or send me an email.

mjsottile commented 3 years ago

@andreasmang @tjasaki I’d like to not get blocked on install issues if it boils down to CUDA related problems. The difficulty in installing tools based on CUDA is often more an issue with how NVidia chooses to distribute their software/drivers and not a reflection of the software that uses it. I would recommend @tjasaki communicate directly with the CLAIRE developers via GitHub issues or email with the specific problems being encountered on installation, and if the issue can’t be resolved, skip the installation part of the review for now. I don’t think it’s useful if we get bogged down in third party GPU driver/framework problems that are orthogonal to the package under review. If everything else looks reasonable on review, we can revisit whether more needs to be done related to installation at the end of the refereeing process.

tjasaki commented 3 years ago

@mjsottile: Check. I've asked for tangential assistance, but will proceed as I am able.

mjsottile commented 3 years ago

:wave: @NMontanaBrown and @tjasaki This review appears to have stalled and I want to check to make sure you are both able to continue to make progress. Please let me or the authors of the submission know if you are blocked and need assistance to move things along.

mjsottile commented 3 years ago

@andreasmang apologies for the review stalling out. I've reached out to the reviewers off of GitHub and if I don't see progress soon I'll be looking for alternative reviewers.

tjasaki commented 3 years ago

@mjsottile I spent some time this weekend with the code, its operation, and the review. I am quite satisfied and have completed my checklist. It is a neat piece of work. What is my next step?

mjsottile commented 3 years ago

Thanks @tjasaki. I took a look at the issues opened on the project and saw the exchange related to installation. Glad it was resolved. I’ll consider your review complete. The other reviewer responded over email and has been consumed by other projects, so I’ll take over in their place and do my own review instead of looking for an additional reviewer. Once I complete my review we’ll be able to move on to the next phase of the paper process.

mjsottile 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:

mjsottile 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.1007/978-3-319-66182-7_32 is OK
- 10.1038/nprot.2014.123 is OK
- 10.1023/A:1008001603737 is OK
- 10.1007/978-3-642-12055-8 is OK
- 10.1023/B:VISI.0000043755.93987.aa is OK
- 10.1146/annurev-bioeng-062117-121105 is OK
- 10.1109/TMI.2013.2265603 is OK
- 10.1137/1.9780898718843 is OK
- 10.1088/0266-5611/24/3/034008 is OK
- 10.1093/acprof:oso/9780198528418.001.0001 is OK
- 10.1007/978-3-030-59713-9_53 is OK
- 10.1137/19M1275280 is OK
- 10.1109/TMI.2020.3024264 is OK
- 10.1016/j.cma.2018.12.008 is OK
- 10.1007/s11081-018-9390-9 is OK
- 10.1109/SC41405.2020.00042 is OK
- 10.1016/j.jpdc.2020.11.006 is OK
- 10.1137/18M1207818 is OK
- 10.1145/3126908.3126930 is OK
- 10.1137/140984002 is OK
- 10.1137/15m1010919 is OK
- 10.1137/16m1070475 is OK
- 10.1109/sc.2016.71 is OK

MISSING DOIs

- None

INVALID DOIs

- None
mjsottile commented 3 years ago

@whedon remove @NMontanaBrown as reviewer

whedon commented 3 years ago

OK, @NMontanaBrown is no longer a reviewer

mjsottile commented 3 years ago

@andreasmang I've looked over the submission myself in addition to the review from @tjasaki and I am satisfied that it meets the requirements for JOSS. The last steps:

Once you've provided that information I can issue the final commands to pass the submission to an EIC for final processing.

andreasmang 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:

andreasmang commented 3 years ago

@mjsottile I have looked carefully over the submission.

Let me know if further action is required on my end and thanks in advance for your help.

andreasmang commented 3 years ago

Here's a link that should work: https://zenodo.org/record/4837427

mjsottile commented 3 years ago

@whedon set 10.5281/zenodo.4837427 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.4837427 is the archive.

mjsottile commented 3 years ago

@whedon set v0.08 as version

whedon commented 3 years ago

OK. v0.08 is the version.

mjsottile commented 3 years ago

@whedon accept

whedon commented 3 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1007/978-3-319-66182-7_32 is OK
- 10.1038/nprot.2014.123 is OK
- 10.1023/A:1008001603737 is OK
- 10.1007/978-3-642-12055-8 is OK
- 10.1023/B:VISI.0000043755.93987.aa is OK
- 10.1146/annurev-bioeng-062117-121105 is OK
- 10.1109/TMI.2013.2265603 is OK
- 10.1137/1.9780898718843 is OK
- 10.1088/0266-5611/24/3/034008 is OK
- 10.1093/acprof:oso/9780198528418.001.0001 is OK
- 10.1007/978-3-030-59713-9_53 is OK
- 10.1137/19M1275280 is OK
- 10.1109/TMI.2020.3024264 is OK
- 10.1016/j.cma.2018.12.008 is OK
- 10.1007/s11081-018-9390-9 is OK
- 10.1109/SC41405.2020.00042 is OK
- 10.1016/j.jpdc.2020.11.006 is OK
- 10.1137/18M1207818 is OK
- 10.1145/3126908.3126930 is OK
- 10.1137/140984002 is OK
- 10.1137/15m1010919 is OK
- 10.1137/16m1070475 is OK
- 10.1109/sc.2016.71 is OK

MISSING DOIs

- None

INVALID DOIs

- None
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/2346

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

@whedon accept deposit=true
arfon commented 3 years ago

@whedon accept

whedon commented 3 years ago

To recommend a paper to be accepted use @whedon recommend-accept

arfon commented 3 years ago

@whedon recommend-accept

whedon commented 3 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1007/978-3-319-66182-7_32 is OK
- 10.1038/nprot.2014.123 is OK
- 10.1023/A:1008001603737 is OK
- 10.1007/978-3-642-12055-8 is OK
- 10.1023/B:VISI.0000043755.93987.aa is OK
- 10.1146/annurev-bioeng-062117-121105 is OK
- 10.1109/TMI.2013.2265603 is OK
- 10.1137/1.9780898718843 is OK
- 10.1088/0266-5611/24/3/034008 is OK
- 10.1093/acprof:oso/9780198528418.001.0001 is OK
- 10.1007/978-3-030-59713-9_53 is OK
- 10.1137/19M1275280 is OK
- 10.1109/TMI.2020.3024264 is OK
- 10.1016/j.cma.2018.12.008 is OK
- 10.1007/s11081-018-9390-9 is OK
- 10.1109/SC41405.2020.00042 is OK
- 10.1016/j.jpdc.2020.11.006 is OK
- 10.1137/18M1207818 is OK
- 10.1145/3126908.3126930 is OK
- 10.1137/140984002 is OK
- 10.1137/15m1010919 is OK
- 10.1137/16m1070475 is OK
- 10.1109/sc.2016.71 is OK

MISSING DOIs

- None

INVALID DOIs

- None