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]: MemCNN: a Framework for Developing Memory Efficient Invertible Neural Networks #1576

Closed whedon closed 5 years ago

whedon commented 5 years ago

Submitting author: @silvandeleemput (Sil C. van de Leemput) Repository: https://github.com/silvandeleemput/memcnn Version: 0.3.3 Editor: @terrytangyuan Reviewer: @GregaVrbancic, @sepandhaghighi Archive: 10.5281/zenodo.3353604

Status

status

Status badge code:

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

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

@GregaVrbancic & @sepandhaghighi, 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 @terrytangyuan know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @GregaVrbancic

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @sepandhaghighi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 5 years ago

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

: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 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

sepandhaghighi commented 5 years ago

@whedon check references

whedon commented 5 years ago 
Attempting to check references...
whedon commented 5 years ago 

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1109/cvpr.2016.90 may be missing for title: Deep Residual Learning for Image Recognition
- https://doi.org/10.1109/cvprw.2009.5206848 may be missing for title: ImageNet: A Large-Scale Hierarchical Image Database
- https://doi.org/10.1007/978-3-642-35289-8_27 may be missing for title: Training deep and recurrent networks with hessian-free optimization
- https://doi.org/10.1109/iccv.2017.244 may be missing for title: Unpaired image-to-image translation using cycle-consistent adversarial networks

INVALID DOIs

- None
sepandhaghighi commented 5 years ago

review in progress, would say "accept with minor revisions". added 4 issues, after those are closed, this should be ready to accept silvandeleemput/memcnn/issues/22 silvandeleemput/memcnn/issues/21 silvandeleemput/memcnn/issues/20 silvandeleemput/memcnn/issues/19

sepandhaghighi commented 5 years ago

@whedon check references

whedon commented 5 years ago 
Attempting to check references...
whedon commented 5 years ago 

OK DOIs

- 10.1007/978-3-319-46493-0_38 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/cvprw.2009.5206848 is OK
- 10.1007/978-3-642-35289-8_27 is OK
- 10.1109/iccv.2017.244 is OK

MISSING DOIs

- None

INVALID DOIs

- None
sepandhaghighi commented 5 years ago

another suggestion ;-) silvandeleemput/memcnn/issues/23

sepandhaghighi commented 5 years ago

@silvandeleemput Thanks for your efforts! @terrytangyuan Looks good to me

terrytangyuan commented 5 years ago

Thanks @sepandhaghighi. @GregaVrbancic Any update on the remaining items in the review checklist?

GregaVrbancic commented 5 years ago

Hi @silvandeleemput @terrytangyuan, sorry for a bit of a delay, I was swamped at work. I finally had time to go through the remaining items from the checklist.

In addition to the already addressed remarks from @sepandhaghighi, @silvandeleemput please take a look at this issue: https://github.com/silvandeleemput/memcnn/issues/24

GregaVrbancic commented 5 years ago

@terrytangyuan I'm satisfied with the changes made. It looks good to me for publishing.

terrytangyuan commented 5 years ago

Thank you!

@silvandeleemput - At this point could you make a new release of this software that includes the changes that have resulted from this review. Then, please make an archive of the software in Zenodo/figshare/other service and update this thread with the DOI of the archive? For the Zenodo/figshare archive, please make sure that:

I can then move forward with accepting the submission.

silvandeleemput commented 5 years ago

@sepandhaghighi @GregaVrbancic Thank you both for your time and effort in reviewing this work.

@terrytangyuan I have created a new release of MemCNN (1.0.0) with the changes from the review included and I have uploaded it to Zenodo and saved it as an unpublished upload. The DOI of the archive is: 10.5281/zenodo.3353604. The title and the authors of the submission are the same as in the paper and should match your requirements.

terrytangyuan commented 5 years ago

@whedon set 10.5281/zenodo.3353604 as archive

whedon commented 5 years ago

OK. 10.5281/zenodo.3353604 is the archive.

terrytangyuan commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
terrytangyuan commented 5 years ago

@whedon set 0.3.3 as version

whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

whedon commented 5 years ago

OK. 0.3.3 is the version.

terrytangyuan commented 5 years ago

@whedon accept

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

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

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

@whedon accept deposit=true
terrytangyuan commented 5 years ago

@openjournals/joss-eics Since this is the first accepted paper I am editing, could you help double check? Also, it looks like @whedon accept deposit=true can only be done by editors-in-chief so I am handing this over to you now.

kyleniemeyer commented 5 years ago

@terrytangyuan sure thing. This is the right point to tag us; I do the following:

Also, an explicit @whedon check references check at the end is helpful, but is not a perfect tool.

The AEiCs will always do these checks when you hand it off to us, but it's certainly helpful for you to take care of any such issues before it gets to us.

kyleniemeyer commented 5 years ago

@whedon check references

whedon commented 5 years ago 
Attempting to check references...
whedon commented 5 years ago 

OK DOIs

- 10.1007/978-3-319-46493-0_38 is OK
- 10.1109/cvpr.2016.90 is OK
- 10.1109/cvprw.2009.5206848 is OK
- 10.1007/978-3-642-35289-8_27 is OK
- 10.1109/iccv.2017.244 is OK

MISSING DOIs

- None

INVALID DOIs

- None
silvandeleemput commented 5 years ago

@kyleniemeyer @terrytangyuan The Zenodo code archive is now published (MemCNN version 1.0.0) and can be found here: https://zenodo.org/record/3353604#.XT6VqOgzaUk

kyleniemeyer commented 5 years ago

Thanks @silvandeleemput; can you merge this PR that corrects some references? https://github.com/silvandeleemput/memcnn/pull/25

I'll be looking through the paper itself next.

silvandeleemput commented 5 years ago

@kyleniemeyer Thanks! I have merged the PR.

kyleniemeyer commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

kyleniemeyer commented 5 years ago

Hi @silvandeleemput, for the paper, I've made some edits to in-text citations in https://github.com/silvandeleemput/memcnn/pull/26.

My only other requested change is for you to add, from our list of "what should my paper contain?":

  • A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience.

Thanks!

silvandeleemput commented 5 years ago

@kyleniemeyer We are unsure what exactly is meant with a "non-specialist audience" since it is very likely that people finding this work are at least familiar with neural networks, Python, and/or PyTorch, which should be sufficient to understand the purpose of the library from the paper.

If we are to add the requested change, what section of the paper do you recommend for this?

kyleniemeyer commented 5 years ago

@silvandeleemput We just ask that all papers try to have a sentence or two at the beginning explaining the purpose in a way such that a reader from a non-expert background could understand (in a shallow way) what the application is.

In your case, you might add one or two sentences at the very beginning explaining classification problems and/or neural networks for a general reader.

silvandeleemput commented 5 years ago

@kyleniemeyer Thank you for clarifying your recommendation. We have added a high-level explanation about neural networks to the summary. Additionally, we have also added a missing arxiv URL to the bibliography so that all works have either an URL or DOI.

kyleniemeyer commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

kyleniemeyer commented 5 years ago

@silvandeleemput looks great to me!

kyleniemeyer commented 5 years ago

@whedon accept

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

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

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

@whedon accept deposit=true
kyleniemeyer commented 5 years ago

@whedon accept deposit=true