search

openjournals / joss-reviews

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

[REVIEW]: pySLM2: A full-stack python package for holographic beam shaping #6315

Open editorialbot opened 7 months ago

editorialbot commented 7 months ago

Submitting author: !--author-handle-->@ldes89150<!--end-author-handle-- (Chung-You Shih) Repository: https://github.com/QITI/pySLM2 Branch with paper.md (empty if default branch): paper Version: v0.1.0 Editor: !--editor-->@HaoZeke<!--end-editor-- Reviewers: @brandondube, @sidihamady, @wavefrontshaping Archive: Pending

Status

status

Status badge code:

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

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

@brandondube & @sidihamady & @ktahar & @wavefrontshaping, 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 @HaoZeke 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 @sidihamady

📝 Checklist for @brandondube

editorialbot commented 7 months 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 7 months ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.01 s (342.9 files/s, 34522.9 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TeX                              1             16              0            159
Markdown                         1             39              0             65
YAML                             1              1              4             18
-------------------------------------------------------------------------------
SUM:                             3             56              4            242
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 7 months ago

Wordcount for paper.md is 1738

editorialbot commented 7 months ago

Failed to discover a valid open source license

editorialbot commented 7 months ago

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

editorialbot commented 7 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.5281/zenodo.6121191 is OK

MISSING DOIs

- 10.1038/s41534-021-00396-0 may be a valid DOI for title: Reprogrammable and high-precision holographic optical addressing of trapped ions for scalable quantum control
- 10.1364/oe.24.013881 may be a valid DOI for title: Ultra-precise holographic beam shaping for microscopic quantum control
- 10.1016/s0079-6638(08)70072-6 may be a valid DOI for title: III computer-generated holograms: Techniques and applications
- 10.1038/srep00721 may be a valid DOI for title: Robust digital holography for ultracold atom trapping
- 10.1103/physrevlett.127.263603 may be a valid DOI for title: Super-resolved imaging of a single cold atom on a nanosecond timescale
- 10.1103/physrevlett.127.143602 may be a valid DOI for title: Optical superresolution sensing of a trapped ion’s wave packet size
- 10.1103/physrevlett.97.170406 may be a valid DOI for title: Quantized rotation of atoms from photons with orbital angular momentum
- 10.1103/physrevlett.78.4713 may be a valid DOI for title: Novel optical trap of atoms with a doughnut beam
- 10.1364/oe.18.017193 may be a valid DOI for title: Multi-focus two-photon polymerization technique based on individually controlled phase modulation
- 10.1364/ol.44.003178 may be a valid DOI for title: Large-scale uniform optical focus array generation with a phase spatial light modulator
- 10.1038/nature15750 may be a valid DOI for title: Measuring entanglement entropy in a quantum many-body system
- 10.1038/nmeth.1241 may be a valid DOI for title: Holographic photolysis of caged neurotransmitters

INVALID DOIs

- None
HaoZeke commented 7 months ago

@ldes89150, sorry about this, but after deliberations with @kyleniemeyer (our Associate Editor-in-Chief) I believe it would be best to remove the section on Hardware Integration instead of leaving it in with [not-reviewed]. The rest of the review can proceed, just without that section.

The alternative (preferred) would be having a reviewer with the requisite hardware, but removing the section works as well.

kyleniemeyer commented 7 months ago

@ldes89150 yes - we can include that section in the paper, if you are able to help us find an editor with access to the hardware.

ldes89150 commented 7 months ago

@kyleniemeyer @HaoZeke Based on the opensource works, a researcher I can think of that may have the right equipment is: https://github.com/wavefrontshaping

I can declare there is no conflict of interest.

HaoZeke commented 7 months ago

hi @wavefrontshaping 👋 would you be interested in and available to review this JOSS submission? We carry out our checklist-driven reviews here in GitHub issues and follow these guidelines: joss.readthedocs.io/en/latest/review_criteria.html

If not, could you recommend any potential reviewers?

wavefrontshaping commented 7 months ago

Hi all, Works for me. I can review this work! Not sure how this works though, I read JOSS review guidelines, but where am I supposed to report my comments? Here?

HaoZeke commented 7 months ago

@editorialbot add @wavefrontshaping as reviewer

Thanks a ton for taking time to review this!

Not sure how this works though, I read JOSS review guidelines, but where am I supposed to report my comments? Here?

The review checklist is generated here by commenting @editorialbot generate my checklist, and the final discussion is here (along with responses to the review) but we also recommend directly opening issues in the repository (mentioning that they're part of a JOSS review.

editorialbot commented 7 months ago

@wavefrontshaping added to the reviewers list!

wavefrontshaping commented 7 months ago

Hi,

I installed the package in a new environment. I tried to build the docs using make html.

First, the dependencies for the docs are not installed when I installed the package, did I do something wrong? I did not find instructions for making the docs in the repo (maybe I missed something), so I add the packages one by one when the errors appeared. It may be useful to a requirement file in the docs folder.

Then, after installing the necessary packages, I run into an error:

Theme error:
no theme named 'furo' found (missing theme.conf?)

How do I solve that?

brandondube commented 7 months ago

👋 @wavefrontshaping FYI, JOSS likes reviewers to use github issues on the being-reviewed repository for their reviews, not post questions, etc in the review issue on JOSS.

ldes89150 commented 7 months ago

Hey @wavefrontshaping ,

Thank you so much for helping the review. I opened an issue on github for the problem you encountered. https://github.com/QITI/pySLM2/issues/10

sidihamady commented 7 months ago

Review checklist for @sidihamady

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

sidihamady commented 7 months ago

Issue: Authors can add a header to each source file

https://github.com/QITI/pySLM2/issues/12

sidihamady commented 7 months ago

Installation: The package is easy to install and the examples provided allow you to test it without problem. The only example that I can't test is obviously the one that uses the Luxbeam controller since I don't have access to the hardware.

sidihamady commented 7 months ago
kyleniemeyer commented 6 months ago

Hello @HaoZeke, @ktahar has asked to step down from reviewing this article. Fortunately, it has more than enough reviewers already.

kyleniemeyer commented 6 months ago

@editorialbot remove @ktahar as reviewer

editorialbot commented 6 months ago

@ktahar removed from the reviewers list!

brandondube commented 6 months ago

Review checklist for @brandondube

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

HaoZeke commented 6 months ago

@ldes89150 just checking in on how the responses are coming along, I see there's some good feedback to be incorporated :)

ldes89150 commented 6 months ago

@HaoZeke

@ldes89150 just checking in on how the responses are coming along, I see there's some good feedback to be incorporated :)

We are currently working on addressing the issues reviewers raised! I will post here once we've done most of the changes.

HaoZeke commented 5 months ago

Hi @ldes89150, just wanted to check in / provide a reminder regarding the reviews.

brandondube commented 4 months ago

It has been over a month since there was any progress on this review; what is the path forward?

HaoZeke commented 4 months ago

@ldes89150 any updates on the progress?

ldes89150 commented 3 months ago

Hey @HaoZeke @brandondube ,

I apologize for the delay. I recently started a new position and have been quite busy, especially with traveling over the past two weeks. I will do my best to wrap things up this week!

HaoZeke commented 3 months ago

@editorialbot remind @ldes89150 in three days

editorialbot commented 3 months ago

Reminder set for @ldes89150 in three days

editorialbot commented 3 months ago

:wave: @ldes89150, please update us on how things are progressing here (this is an automated reminder).

ldes89150 commented 3 months ago

Hey @HaoZeke @wavefrontshaping @brandondube @sidihamady

I believe I have addressed all the issues related to the documents and the code mentioned in the issues. Please take a look!

I still plan to make some changes to the main text of the paper, but this should be completed in a few days. Thank you so much for your time! I really appreciate it.

HaoZeke commented 2 months ago

@editorialbot remind @ldes89150 in three days

editorialbot commented 2 months ago

Reminder set for @ldes89150 in three days

ldes89150 commented 2 months ago

@editorialbot generate pdf

editorialbot commented 2 months ago

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

ldes89150 commented 2 months ago

Happy July 4th!

@HaoZeke The paper has been updated and checked by all three authors.

HaoZeke commented 2 months ago

Thanks @ldes89150, happy fourth ^_^

@brandondube, @sidihamady and @wavefrontshaping, could you please take a look and confirm that the issues raised were addressed?

wavefrontshaping commented 2 months ago

All issues have been addressed, on my side all is good except for some typos to correct (here are some I found, pretty sure I missed some).

line 60: macopixel -> macropixel line 60: DMD -> DMDs line 61-62: for generating DMD hologram gerenation -> for DMD hologram gerenation line 89: two type of -> two types of line 89: I thing all the nouns should be plurals (DMDs, etc.) line 108: As illustrated in Fig. Figure 1 -> As illustrated in Figure 1

sidihamady commented 2 months ago

The authors addessed the issues, modulo the typos noted by my colleague @wavefrontshaping

editorialbot commented 2 months ago

:wave: @ldes89150, please update us on how things are progressing here (this is an automated reminder).

marvelousmonicaaa commented 2 months ago

@editorialbot generate pdf

editorialbot commented 2 months ago

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

marvelousmonicaaa commented 2 months ago

The authors addessed the issues, modulo the typos noted by my colleague @wavefrontshaping

Hi @wavefrontshaping , thanks for pointing the typos issue out! I have now checked and corrected the spellings in the paper and documentations. @ldes89150

HaoZeke commented 1 month ago

@sidihamady @wavefrontshaping @brandondube just checking in on the review, this seems close to the finish line?

brandondube commented 1 month ago

Github shows I left my last comment about 3 weeks ago, and am waiting on reply from the authors

ldes89150 commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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