[REVIEW]: DeconvOptim.jl - Signal Deconvolution with Julia

whedon commented 1 year ago

Submitting author: !--author-handle-->@roflmaostc<!--end-author-handle-- (Felix Wechsler) Repository: https://github.com/roflmaostc/DeconvOptim.jl Branch with paper.md (empty if default branch): Version: v0.7.1 Editor: Reviewers: @sloede, @matbesancon Archive:

Status

Status badge code:

HTML: <a href="https://proceedings.juliacon.org/papers/fbb7d7d0388a8a9f2f97cecc632cf29a"><img src="https://proceedings.juliacon.org/papers/fbb7d7d0388a8a9f2f97cecc632cf29a/status.svg"></a>
Markdown: [![status](https://proceedings.juliacon.org/papers/fbb7d7d0388a8a9f2f97cecc632cf29a/status.svg)](https://proceedings.juliacon.org/papers/fbb7d7d0388a8a9f2f97cecc632cf29a)

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

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

Make sure you're logged in to your GitHub account
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 @vchuravy 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 @sloede

Conflict of interest

[x] As the reviewer I confirm that I have read the JuliaCon conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JuliaCon code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Authorship: Has the submitting author (@roflmaostc) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
[ ] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Paper format

[x] Authors: Does the paper.tex file include a list of authors with their affiliations?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
[x] Page limit: Is the page limit for full papers respected by the submitted document?

Content

[x] Context: is the scientific context motivating the work correctly presented?
[x] Methodology: is the approach taken in the work justified, presented with enough details and reference to reproduce it?
[x] Results: are the results presented and compared to approaches with similar goals?

Review checklist for @matbesancon

Conflict of interest
[x] As the reviewer I confirm that I have read the JuliaCon conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JuliaCon code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Authorship: Has the submitting author (@roflmaostc) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Paper format

[x] Authors: Does the paper.tex file include a list of authors with their affiliations?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
[x] Page limit: Is the page limit for full papers respected by the submitted document?

Content

[x] Context: is the scientific context motivating the work correctly presented?
[x] Methodology: is the approach taken in the work justified, presented with enough details and reference to reproduce it?
[x] Results: are the results presented and compared to approaches with similar goals?

whedon commented 1 year ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @sloede, @matbesancon 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/JuliaCon/proceedings-review) 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:

Set yourself as 'Not watching' https://github.com/JuliaCon/proceedings-review:

watching

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 1 year ago

Failed to discover a Statement of need section in paper

whedon commented 1 year ago

Wordcount for paper.tex is 5009

whedon commented 1 year ago

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.11 s (797.1 files/s, 169039.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TeX                             10            321            179           3069
TOML                             6            594              2           2787
Julia                           29            637            333           2238
SVG                              2              0              0            740
Markdown                        20            198              0            526
Jupyter Notebook                15              0           6355            515
YAML                             4              1              0            108
Ruby                             1              8              4             45
Bourne Shell                     1              1              0              2
-------------------------------------------------------------------------------
SUM:                            88           1760           6873          10030
-------------------------------------------------------------------------------

Statistical information for the repository '1e8976378134c15e41773d26' was
gathered on 2022/06/30.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
roflmaostc                       1            57              0          100.00

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
roflmaostc                   57          100.0          0.0                7.02

whedon commented 1 year ago

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

whedon commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1364/JOSA.62.000055 is OK
- 10.1086/111605 is OK
- 10.21105/joss.00615 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1364/JOSAA.15.001077 is OK
- 10.1145/1401132.1401152 is OK
- 10.1017/9781108552660 is OK
- 10.5281/zenodo.5047410 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.5334/jors.151 is OK

MISSING DOIs

- 10.1137/141000671 may be a valid DOI for title: Julia: A fresh approach to numerical computing
- 10.1109/iccv.2017.491 may be a valid DOI for title: Learning to Push the Limits of Efficient FFT-Based Image Deconvolution
- 10.1109/jproc.2004.840301 may be a valid DOI for title: The Design and Implementation of FFTW3
- 10.1007/bf01589116 may be a valid DOI for title: On the Limited Memory BFGS Method for Large Scale Optimization
- 10.2307/2334515 may be a valid DOI for title: Nonparametric Roughness Penalties for Probability Densities
- 10.1016/j.ymeth.2016.12.015 may be a valid DOI for title: DeconvolutionLab2: An Open-Source Software for Deconvolution Microscopy
- 10.1038/s41598-018-29768-x may be a valid DOI for title: A convex 3D deconvolution algorithm for low photon count fluorescence imaging
- 10.1016/j.advengsoft.2019.02.002 may be a valid DOI for title: Rapid software prototyping for heterogeneous and distributed platforms

INVALID DOIs

- https://doi.org/10.1016/j.micron.2006.07.009 is INVALID because of 'https://doi.org/' prefix

whedon commented 1 year ago

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

whedon commented 1 year ago

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

sloede commented 1 year ago

Summary of the paper

The manuscript describes the software package DeconvOptim.jl, a toolbox written in Julia for the deconvolution of measured signals that have been degraded by blurring and/or Gaussian noise/Poisson shot noise. It gives an introduction to the pertinent basics of digital signal processing based on deconvolution, and discusses some of the common design choices and issues when using this approach. It then describes the pipeline of the deconvolution approach used in DeconvOptim.jl. Finally, experiments with one- and multi-dimensional data sets are shown, where an emphasis is put on the influence of regularization. Further, performance comparisons to existing deconvolution tools are shown, demonstrating that the presented software package is ahead of or on par with most of the other tools.

Review

Overall, the paper is well written, with a clear storyline, a helpful introduction for readers not familiar with deconvolution techniques, and useful examples in the experiment section. There are only a few minor issues that I noted. Once these are adequately addressed, I think this paper can be accepted for publication (minor modification request):

a) p. 1, sec. 2: "...we also need an efficient way..."

b) p. 3, bottom right: It is unfortunate that the code listing is split across two pages. I strongly recommend to reformat its placement such that it is shown completely on a single page.

c) p. 4, sec. 5: The section introduction once mentions Julia 1.6.1 and once Julia 1.6.2, thus it is not clear which version was used.

d) p. 4, sec. 5.2: "Figure 5c shows the result after 50 iterations..." --> I believe this should read "Figure 5b shows the..."

e) p. 4, sec. 5.2: "We see [...] heavily affected by some artifacts [...]" and later "...it can be seen visually as well.": In both cases, it is not clear to me what exactly these artifacts are and what "it" is that can be seen visually. Since this is a qualitative analysis and not a quantitative one, I think the authors should offer considerably more detail on which particular features they make their observations that one approach is better than the other. Further, the qualitative analysis should be augmented by showing the original ("ground truth") image in Fig. 5 along with the convolved images.

f) p. 4, fig. 4 and p. 5, fig. 6: Both these images seem to have issues with the correct labeling of the axes and the legends (i.e., symbols and digits are missing). As they are right now, it is hard to fully understand them.

g) p. 5, sec. 5.3: It is not obvious to me why it is a four-dimensional dataset when talking about a 3D image. If it the color channels represent the fourth dimension, I suggest to explicitly say so.

h) p. 5, sec. 5.4.1: "...being able to use of all threads...": I think the "of" is erroneous and should be removed.

i) p. 6, tab. 1: It is not clear from the table description alone what the difference is between the regular measurements and the ones with the nabla sign prefixed. This should be improved such that the table can be understood without having to consult the text. Furthermore, it would be nice if the measurement setup could be briefly described, e.g., how many measurements were taken for each data point, if outliers were removed etc.

j) p. 6, sec. 5.5, tab. 2: For easier visual comparison of the performance numbers, it would be helpful to right-align all time measurements and print them with the same number of digits after the decimal point.

k) p. 6, sec. 6: "... allow to generate regularizers which have ..."

l) p. 7f., sec. 8: Many of the references are lacking DOIs (as requested by the checklist above). Further, most of them have titles that are not properly capitalized, e.g., "fft" instead of "FFT".

roflmaostc commented 1 year ago

Hi, thanks for the detailed feedback! I incorporated it and hopefully it's more clear what we were trying to say

f) this seems to be an issue with @whedon in combination with pgfpots again. Locally, the axis is ok. j) We tried to always show three significant digits but I see the point of the alignment. I'm not sure how to properly solve that.

l) the automatic capitalization seems to happen by the bibtex settings. I tried to fix with with {FFT} in the ref.bib

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

PDF failed to compile for issue #99 with the following error:

 Latexmk: This is Latexmk, John Collins, 17 Jan. 2018, version: 4.55.
Rule 'pdflatex': Rules & subrules not known to be previously run:
   pdflatex
Rule 'pdflatex': The following rules & subrules became out-of-date:
      'pdflatex'
------------
Run number 1 of rule 'pdflatex'
------------
------------
Running 'pdflatex  -recorder  "paper.tex"'
------------
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: List of undefined refs and citations:
  Citation `BenchmarkTools.jl-2016' on page 4 undefined on input line 320
  Citation `CUDA' on page 1 undefined on input line 90
  Citation `DifferentialEquations.jl-2017' on page 4 undefined on input line 319
  Citation `FFTW05' on page 1 undefined on input line 83
  Citation `FFTW05' on page 4 undefined on input line 320
  Citation `Good:71' on page 3 undefined on input line 265
  Citation `Kruse_2017_ICCV' on page 1 undefined on input line 54
  Citation `LBFGS' on page 2 undefined on input line 168
  Citation `LLVM.jl-2017' on page 2 undefined on input line 165
  Citation `LLVM.jl-2017' on page 4 undefined on input line 320
  Citation `Richardson:72' on page 1 undefined on input line 50
  Citation `Verveer:98' on page 3 undefined on input line 265
  Citation `Zygote.jl-2018' on page 2 undefined on input line 167
  Citation `Zygote.jl-2018' on page 4 undefined on input line 321
  Citation `besard2018juliagpu' on page 2 undefined on input line 165
  Citation `bezanson2017julia' on page 1 undefined on input line 56
  Citation `lucy:74' on page 1 undefined on input line 50
  Citation `michael_abbott_2021_5047410' on page 3 undefined on input line 282
  Citation `mogensen2018optim' on page 2 undefined on input line 168
  Citation `mogensen2018optim' on page 4 undefined on input line 320
  Citation `wiener2013extrapolation' on page 1 undefined on input line 51
  Reference `eq:fftconv' on page 1 undefined on input line 84
  Reference `eq:gradpoisson' on page 3 undefined on input line 259
  Reference `eq:poisson_loss' on page 3 undefined on input line 259
  Reference `fig:1D_dataset' on page 4 undefined on input line 325
  Reference `fig:1D_datasetb' on page 4 undefined on input line 345
  Reference `fig:1D_datasetb' on page 4 undefined on input line 354
  Reference `fig:fabioa' on page 4 undefined on input line 360
  Reference `fig:pipeline' on page 2 undefined on input line 146
  Reference `fig:withoutpad' on page 2 undefined on input line 95
  Reference `fig:withpad' on page 2 undefined on input line 100
  Reference `fig:wrap_around' on page 3 undefined on input line 195
  Reference `sec:wrap' on page 2 undefined on input line 138
Latexmk: Summary of warnings:
  Latex failed to resolve 13 reference(s)
  Latex failed to resolve 24 citation(s)
Rule 'pdflatex': File changes, etc:
   Changed files, or newly in use since previous run(s):
      'paper.aux'
------------
Run number 2 of rule 'pdflatex'
------------
------------
Running 'pdflatex  -recorder  "paper.tex"'
------------
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: List of undefined refs and citations:
  Citation `BenchmarkTools.jl-2016' on page 4 undefined on input line 320
  Citation `CUDA' on page 1 undefined on input line 90
  Citation `DifferentialEquations.jl-2017' on page 4 undefined on input line 319
  Citation `FFTW05' on page 1 undefined on input line 83
  Citation `FFTW05' on page 4 undefined on input line 320
  Citation `Good:71' on page 3 undefined on input line 265
  Citation `Kruse_2017_ICCV' on page 1 undefined on input line 54
  Citation `LBFGS' on page 2 undefined on input line 168
  Citation `LLVM.jl-2017' on page 2 undefined on input line 165
  Citation `LLVM.jl-2017' on page 4 undefined on input line 320
  Citation `Richardson:72' on page 1 undefined on input line 50
  Citation `Verveer:98' on page 3 undefined on input line 265
  Citation `Zygote.jl-2018' on page 2 undefined on input line 167
  Citation `Zygote.jl-2018' on page 4 undefined on input line 321
  Citation `besard2018juliagpu' on page 2 undefined on input line 165
  Citation `bezanson2017julia' on page 1 undefined on input line 56
  Citation `lucy:74' on page 1 undefined on input line 50
  Citation `michael_abbott_2021_5047410' on page 3 undefined on input line 282
  Citation `mogensen2018optim' on page 2 undefined on input line 168
  Citation `mogensen2018optim' on page 4 undefined on input line 320
  Citation `wiener2013extrapolation' on page 1 undefined on input line 51
  Reference `fig:1D_dataset' on page 4 undefined on input line 325
  Reference `fig:1D_datasetb' on page 4 undefined on input line 345
  Reference `fig:1D_datasetb' on page 4 undefined on input line 354
  Reference `fig:fabioa' on page 4 undefined on input line 360
Latexmk: Summary of warnings:
  Latex failed to resolve 4 reference(s)
  Latex failed to resolve 24 citation(s)
Rule 'pdflatex': File changes, etc:
   Changed files, or newly in use since previous run(s):
      'paper.out'
------------
Run number 3 of rule 'pdflatex'
------------
------------
Running 'pdflatex  -recorder  "paper.tex"'
------------
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 3'
===========Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: Missing input file: 'figures/resolution_test_512.png' from line
  'LaTeX Warning: File `figures/resolution_test_512.png' not found on input line 364.'
Latexmk: List of undefined refs and citations:
  Citation `BenchmarkTools.jl-2016' on page 4 undefined on input line 320
  Citation `CUDA' on page 1 undefined on input line 90
  Citation `DifferentialEquations.jl-2017' on page 4 undefined on input line 319
  Citation `FFTW05' on page 1 undefined on input line 83
  Citation `FFTW05' on page 4 undefined on input line 320
  Citation `Good:71' on page 3 undefined on input line 265
  Citation `Kruse_2017_ICCV' on page 1 undefined on input line 54
  Citation `LBFGS' on page 2 undefined on input line 168
  Citation `LLVM.jl-2017' on page 2 undefined on input line 165
  Citation `LLVM.jl-2017' on page 4 undefined on input line 320
  Citation `Richardson:72' on page 1 undefined on input line 50
  Citation `Verveer:98' on page 3 undefined on input line 265
  Citation `Zygote.jl-2018' on page 2 undefined on input line 167
  Citation `Zygote.jl-2018' on page 4 undefined on input line 321
  Citation `besard2018juliagpu' on page 2 undefined on input line 165
  Citation `bezanson2017julia' on page 1 undefined on input line 56
  Citation `lucy:74' on page 1 undefined on input line 50
  Citation `michael_abbott_2021_5047410' on page 3 undefined on input line 282
  Citation `mogensen2018optim' on page 2 undefined on input line 168
  Citation `mogensen2018optim' on page 4 undefined on input line 320
  Citation `wiener2013extrapolation' on page 1 undefined on input line 51
  Reference `fig:1D_dataset' on page 4 undefined on input line 325
  Reference `fig:1D_datasetb' on page 4 undefined on input line 345
  Reference `fig:1D_datasetb' on page 4 undefined on input line 354
  Reference `fig:fabioa' on page 4 undefined on input line 360
Latexmk: Summary of warnings:
  Latex failed to resolve 4 reference(s)
  Latex failed to resolve 24 citation(s)
Failure to make 'paper.pdf'
Collected error summary (may duplicate other messages):
  pdflatex: Command for 'pdflatex' gave return code 1
      Refer to 'paper.log' for details
Looks like we failed to compile the PDF

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

This is how it looks on my Ubuntu machine. paper.pdf

sloede commented 1 year ago

f) this seems to be an issue with @whedon in combination with pgfpots again. Locally, the axis is ok.

We have had a similar issue with our submission last year. Maybe you can just export the files as PDFs and include them? Also, the code listing is still distributed over 2 pages.

roflmaostc commented 1 year ago

Fixed the code listing. But what puzzles me is that I already exported that figure as pdf and don't use pgfplots for those plots :confused:

I mean, I can convert them to .png which is not a nice solution IMO.

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

Include .png instead @whedon generate pdf

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

The bot also compiles the Bibliography slight differently and hence there is suddenly a new page

sloede commented 1 year ago

f) this seems to be an issue with @whedon in combination with pgfpots again. Locally, the axis is ok.

It looks ok now.

j) We tried to always show three significant digits but I see the point of the alignment. I'm not sure how to properly solve that.

Not sure why you'd require three significant digits for the measurement and wouldn't allow more. Shouldn't a measurement with a given temporal accuracy yield more significant digits if the measured time is increased? That is, if I measure with a sub-millisecond resolution, shouldn't I be able to show measurements with a fixed amount of digits after the decimal point (e.g., 2), no matter if the total interval is one second or 1000 seconds? Anyhow, it's ok as it is if you want to leave it this way.

l) the automatic capitalization seems to happen by the bibtex settings. I tried to fix with with {FFT} in the ref.bib

References 2 and 3 are duplicated. Ref 7 should read "Program Generation", not "Pr1ogram Generation". Ref 17 still has capitalization errors. None of the references is equipped with a DOI, as required by the checklist above.

roflmaostc commented 1 year ago

References 2 and 3 are duplicated. Ref 7 should read "Program Generation", not "Pr1ogram Generation". Ref 17 still has capitalization errors. None of the references is equipped with a DOI, as required by the checklist above.

Well spotted, that was generated by PkgCite.jl but I didn't check it properly, thanks!

I'm not sure why the DOI is not printed but I included it for all references: https://github.com/roflmaostc/DeconvOptim.jl/blob/master/paper/ref.bib

I couldn't spot special settings in recent papers but maybe my juliacon.cls is old?

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

Ah, your juliacon.bst looks different.

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

sloede commented 1 year ago

@vchuravy all my comments have been addressed adequately and from my side I recommend to accept the manuscript.

roflmaostc commented 1 year ago

Is the second (@matbesancon) review still missing?

matbesancon commented 1 year ago

hi, yes indeed it totally slipped out of my radar, sorry for that

matbesancon commented 1 year ago

@whedon remind @matbesancon in 1 week

whedon commented 1 year ago

Reminder set for @matbesancon in 1 week

whedon commented 1 year ago

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

matbesancon commented 1 year ago

Sorry about the delay, here is my review: Notation: detail each term in the beginning (what is r, I).

The mathematical definition of a convolution: convolution here is defined on a univariate domain, it would be good to: define the domain, what S and h are here.

"If one applies a FFT on a finite data set", not sure one would consider one image as a data set, on a finite object maybe?

The experiment summarized in Table 1 seems a bit limited as a comparison, it would be better to have several comparisons aggregated for different sizes (it could be that some methods perform better on small arrays compared to larger ones). This is especially critical since one of the contributions claimed in the abstract is about performance of DeconvOptim.

The LLVM citation seems broken "LLMV [?]"

In the paragraph starting with "As typical for Julia packages", it would be good to replace "code" with more specific terms. "all code is written in" -> the entire package is written in Julia.

Overall, I'm not sure the sentence on typical Julia packages being written in Julia is necessary here. It also implies that FFTW and CUDA are the only packages that do not contain only pure Julia.

Does the package allow for other optimizing methods from Optim.jl? Have the authors tried some of them?

Instead of detailing the packages used for the experiments, list what they were used for.

The lines of Figure 6b are too thin to read well.

Fig 8 takes up a lot of space, could it be resized to fit images two by two?

Double-check the grammar, there were minor improvements and corrections to do: "spatially based convolutions" -> spatial-based?

"In many frameworks such" -> "In many frameworks, such"

"Apart from forward model" -> "Apart from the forward model"

"A (smoothed) Total variation" -> A (smoothed) Total Variation

The authors advocate for letting autodiff compute gradients for flexibility. Have they measured the impact of autodiff versus manual (in-place) gradient computations? Could the package let the user opt out of AD by providing a gradient themselves?

roflmaostc commented 1 year ago

Thanks for your valuable input!

Sorry about the delay, here is my review: Notation: detail each term in the beginning (what is r, I).

done

The mathematical definition of a convolution: convolution here is defined on a univariate domain, it would be good to: define the domain, what S and h are here.

I'm not quite sure what to put here since this seems to be rather lengthy? https://en.wikipedia.org/wiki/Convolution#Domain_of_definition

"If one applies a FFT on a finite data set", not sure one would consider one image as a data set, on a finite object maybe?

changed it

The experiment summarized in Table 1 seems a bit limited as a comparison, it would be better to have several comparisons aggregated for different sizes (it could be that some methods perform better on small arrays compared to larger ones). This is especially critical since one of the contributions claimed in the abstract is about performance of DeconvOptim.

I could add another size? Maybe a 300x300 2D array?

The LLVM citation seems broken "LLMV [?]"

fixed.

In the paragraph starting with "As typical for Julia packages", it would be good to replace "code" with more specific terms. "all code is written in" -> the entire package is written in Julia.

Overall, I'm not sure the sentence on typical Julia packages being written in Julia is necessary here. It also implies that FFTW and CUDA are the only packages that do not contain only pure Julia.

Does the package allow for other optimizing methods from Optim.jl? Have the authors tried some of them?

Yes we, have a keyword opt to specify the optimizer. For convolution, the optimizer LBFGS seems to be fastest and preferred option in the community. We can only confirm this. See here

Instead of detailing the packages used for the experiments, list what they were used for.

Honestly, I generated the citation with PkgCite.jl and some of the packages are just dependencies from other packages. Sometimes, I can't tell exactly what's their functionality in the full code base.

The lines of Figure 6b are too thin to read well.

Agree, happened because I had to change everything to .png instead of pgfplots. Fixed

Fig 8 takes up a lot of space, could it be resized to fit images two by two?

I agree, but I couldn't find a better solution given the image width.

Double-check the grammar, there were minor improvements and corrections to do: "spatially based convolutions" -> spatial-based?

"In many frameworks such" -> "In many frameworks, such"

"Apart from forward model" -> "Apart from the forward model"

"A (smoothed) Total variation" -> A (smoothed) Total Variation

Fixed them

The authors advocate for letting autodiff compute gradients for flexibility. Have they measured the impact of autodiff versus manual (in-place) gradient computations? Could the package let the user opt out of AD by providing a gradient themselves?

Currently not directly possible, but there is the more flexible method generic_invert where one could use own forward models and register the gradients with ChainRules. That works and I did that in the past.

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

I don't really know what's going on with this bot but the compiled layout is different on my machine.... paper.pdf

matbesancon commented 1 year ago

Honestly, I generated the citation with PkgCite.jl and some of the packages are just dependencies from other packages. Sometimes, I can't tell exactly what's their functionality in the full code base.

I can understand the utility to extract them like that but I think it really should be written and phrased properly afterwards or remove them if there is nothing directly relevant about them for the paper and/or package. People can always look up the package and see the dependencies for themselves

roflmaostc commented 1 year ago

Yeah, I agree to that.

I removed the sentence since most of the important packages were already cited before.

@vchuravy I still don't really know what's going on with @whedon. Which LaTeX version is it using? Is it a really old one? The compiled PDFs looks considerably different to the ones on my machine (pdfTeX 3.14159265-2.6-1.40.20 (TeX Live 2019/Debian))

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

roflmaostc commented 1 year ago

Interesting, this one looks very similar to the one on my machine :D

roflmaostc commented 1 year ago

Just wanted to ping again. Anything more to do?

Best, Felix

roflmaostc commented 1 year ago

Hi, What's the state? I think the recent version looks ok in terms of formatting.

Tagging @RainerHeintzmann.

roflmaostc commented 1 year ago

@whedon generate pdf

whedon commented 1 year ago

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

JuliaCon / proceedings-review