[REVIEW]: adrt: approximate discrete Radon transform for Python

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@karlotness@diehlpk<!--end-editor-- Reviewers: @zhangjy-ge, @fruzsinaagocs Archive: 10.5281/zenodo.7738254

Status

Status badge code:

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

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

@zhangjy-ge & @fruzsinaagocs, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @diehlpk 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 ✨

Checklists

📝 Checklist for @zhangjy-ge

📝 Checklist for @fruzsinaagocs

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

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

@editorialbot generate pdf

editorialbot commented 1 year ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.11 s (739.9 files/s, 107020.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          32            779           1685           2822
C++                             14            183            537           1490
C/C++ Header                     6            151            278            871
reStructuredText                17            404            565            705
YAML                             3              0              3            494
TeX                              1             16              0            151
Markdown                         2             24              0            120
TOML                             1             11              3             76
INI                              1              5              0             47
PowerShell                       1              1              0              4
SVG                              1              0              0              2
-------------------------------------------------------------------------------
SUM:                            79           1574           3071           6782
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 1 year ago

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

OK DOIs

- 10.1137/1.9780898719284 is OK
- 10.1137/S0097539793256673 is OK
- 10.1016/j.aml.2019.106159 is OK
- 10.1073/pnas.0609228103 is OK
- 10.1016/0031-3203(95)00057-7 is OK
- 10.1137/060650301 is OK
- 10.1109/TASSP.1987.1165108 is OK
- 10.1137/S003614450343200X is OK
- 10.1137/18M120885X is OK
- 10.1137/17M1135633 is OK
- 10.1090/mcom/3750 is OK
- 10.7717/peerj.453 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1109/MCSoC.2015.38 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

Wordcount for paper.md is 586

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

zhangjy-ge commented 1 year ago

Review checklist for @zhangjy-ge

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

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

General checks

[x] Repository: Is the source code for this software available at the https://github.com/karlotness/adrt?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@karlotness) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

karlotness commented 1 year ago

Thank you both for being willing to review. If you have any questions, please let us know; we'll be happy to answer.

diehlpk commented 1 year ago

Hi @zhangjy-ge, @fruzsinaagocs how is your review going?

zhangjy-ge commented 1 year ago

Hi @diehlpk I am still in the procedure of reviewing it. Kind of busy this week but I should have some updates next week.

zhangjy-ge commented 1 year ago

Hi @karlotness I have finished my preliminary review. I think it is a useful and well documented package based on my reading of your examples/tutorials/papers. However, I have some problems installing the software thus I cannot proceed to judge its functionality. I have submitted my errors in github issues.

karlotness commented 1 year ago

Thanks, much appreciated. Thanks also for filing the issues. Hopefully we can work out what's going wrong there and get those fixed up.

zhangjy-ge commented 1 year ago

Hi @diehlpk @karlotness I have finished my review. We have figured out that the installation issue is not from adrt so I have updated my checklist. I would say adrt is a well-designed package with sufficient endeavor and the manuscript itself is good for publication. Thanks for the opportunity to review the manuscript.

karlotness commented 1 year ago

Glad to hear it. Thanks for helping check out those problems, and thanks again for your willingness to review!

fruzsinaagocs commented 1 year ago

Review checklist for @fruzsinaagocs

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

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

General checks

[x] Repository: Is the source code for this software available at the https://github.com/karlotness/adrt?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@karlotness) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

diehlpk commented 1 year ago

@fruzsinaagocs how is your review going?

fruzsinaagocs commented 1 year ago

Hi @diehlpk, I just want to check a few more things (related to the scaling claims), and I'll be done. I think I can get it done by the end of the week, if that's OK.

diehlpk commented 1 year ago

@fruzsinaagocs sure, take your time, I just wanted to check how things are going.

fruzsinaagocs commented 1 year ago

Hi @diehlpk, @karlotness, I finished my preliminary review, and everything looks good. I submitted karlotness/adrt#4 which investigates the time complexity of ADRT, found that the complexity claim was true, with something mildly interesting happening at intermediate image sizes, which I think might be worth commenting on.

diehlpk commented 1 year ago

@karlotness Please address this issue and I will proceed with the editorial check.

fruzsinaagocs commented 1 year ago

@diehlpk, The issue has now been addressed (in short, @karlotness demonstrated that the interesting 'dip' in the complexity curve was driven by cache effects), I'm happy to accept the paper for publication. Thank you for asking me to review!

karlotness commented 1 year ago

Thanks for taking a look and for being willing to review!

diehlpk commented 1 year ago

@editorialbot generate pdf

diehlpk commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago

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

OK DOIs

- 10.1137/1.9780898719284 is OK
- 10.1137/S0097539793256673 is OK
- 10.1016/j.aml.2019.106159 is OK
- 10.1073/pnas.0609228103 is OK
- 10.1016/0031-3203(95)00057-7 is OK
- 10.1137/060650301 is OK
- 10.1109/TASSP.1987.1165108 is OK
- 10.1137/S003614450343200X is OK
- 10.1137/18M120885X is OK
- 10.1137/17M1135633 is OK
- 10.1090/mcom/3750 is OK
- 10.7717/peerj.453 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1109/MCSoC.2015.38 is OK

MISSING DOIs

- None

INVALID DOIs

- None

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

diehlpk commented 1 year ago

@karlotness can you please add the city to the affiliations?

diehlpk commented 1 year ago

@karlotness Would it be much work to add your scaling plot from here (https://github.com/karlotness/adrt/issues/4) to the paper?

karlotness commented 1 year ago

No problem, I think both of those should be incorporated in the updated copy on master. I reformatted the plot just a bit

karlotness commented 1 year ago

@editorialbot set master as branch

editorialbot commented 1 year ago

Done! branch is now master

karlotness commented 1 year ago

@editorialbot generate pdf

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

diehlpk commented 1 year ago

@karlotness If you reference the figure in the text and provided on which CPU you executed the code, I am happy to accept the paper.

karlotness commented 1 year ago

Thanks, that's a good point. I've just added that to the figure caption.

The figure also should be referenced in the text, in the discussion of the time complexity of the ADRT.

karlotness commented 1 year ago

@editorialbot generate pdf

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

diehlpk commented 1 year ago

@karlotness Looks good now.

[x] Please generate a new release including all changes during the review and post the release number here.
[ ] Please upload the release to Zeondo or FigShare and post the DOI here. Note that the title and the authors have to match with the paper.

karlotness commented 1 year ago

Should be set on both of those, I think.

I've added a new release version 1.0.1 with a Git tag of v1.0.1

I added the repository to Zenodo. The DOI for this version is 10.5281/zenodo.7738254 and the DOI for "all versions" there is 10.5281/zenodo.7738253

Thanks!

diehlpk commented 1 year ago

@editorialbot set v1.0.1 as version

editorialbot commented 1 year ago

Done! version is now v1.0.1

diehlpk commented 1 year ago

@editorialbot set 10.5281/zenodo.7738254 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7738254

diehlpk commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago

Attempting dry run of processing paper acceptance...

editorialbot commented 1 year ago

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

OK DOIs

- 10.1137/1.9780898719284 is OK
- 10.1137/S0097539793256673 is OK
- 10.1016/j.aml.2019.106159 is OK
- 10.1073/pnas.0609228103 is OK
- 10.1016/0031-3203(95)00057-7 is OK
- 10.1137/060650301 is OK
- 10.1109/TASSP.1987.1165108 is OK
- 10.1137/S003614450343200X is OK
- 10.1137/18M120885X is OK
- 10.1137/17M1135633 is OK
- 10.1090/mcom/3750 is OK
- 10.7717/peerj.453 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1109/MCSoC.2015.38 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

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

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4058, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 1 year ago

👋 @karlotness - I'm the track editor who will handle the final processing for this submission. While proofreading it, I found a number of small issues, as indicated in https://github.com/karlotness/adrt/pull/5 - Please merge this, or let me know what you disagree with, then we can continue the process of acceptance and publication.

karlotness commented 1 year ago

Thank you very much. Those all made sense, and should be merged now

danielskatz commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago

Doing it live! Attempting automated processing of paper acceptance...

openjournals / joss-reviews