Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: Quantum Instrumentation Control Kit - Defect Arbitrary Waveform Generator (QICK-DAWG): A Quantum Sensing Control Framework for Quantum Defects #6380

Open
editorialbot opened this issue Feb 19, 2024 · 18 comments

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Feb 19, 2024

Submitting author: @egriendeau (Emmeline Riendeau)
Repository: https://github.com/sandialabs/qick-dawg
Branch with paper.md (empty if default branch): paper
Version: 0.1.0
Editor: @phibeck
Reviewers: @14shreyasp, @ktahar, @sidihamady
Archive: Pending

Status

status

Status badge code:

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

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

@14shreyasp & @ktahar & @sidihamady, 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 @phibeck 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 @ktahar

@editorialbot
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Software report:

github.com/AlDanial/cloc v 1.88  T=2.21 s (193.3 files/s, 174946.4 lines/s)
-----------------------------------------------------------------------------------
Language                         files          blank        comment           code
-----------------------------------------------------------------------------------
XML                                 84             44             32         279370
Verilog-SystemVerilog              112           4687           4890          16865
VHDL                                75           2045           5173          16365
Tcl/Tk                              28           1221           1136          14610
Python                              55           3016           5742          10114
JSON                                 2              0              0           3175
SVG                                  9              8              8           2328
Jupyter Notebook                    19              0          10016           2220
Markdown                            15            305              0            746
Perl                                 2            146            132            576
DOS Batch                            2             39              5            231
MATLAB                               5             64             73            198
make                                 2             36              6            171
TeX                                  4              9              0            120
reStructuredText                     4             67             74            104
YAML                                 4             13             35             89
Assembly                             2             11             19             33
CSS                                  1              3              3             15
Bourne Shell                         2              3              5             12
-----------------------------------------------------------------------------------
SUM:                               427          11717          27349         347342
-----------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

@editorialbot
Copy link
Collaborator Author

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

OK DOIs

- 10.1063/5.0076249 is OK
- 10.1145/3529397 is OK
- 10.1109/QCE53715.2022.00123 is OK
- 10.48550/arXiv.2309.17233 is OK
- 10.5281/zenodo.1478113 is OK
- 10.1038/natrevmats.2017.88 is OK
- 10.1088/2633-4356/ace095 is OK
- 10.1063/5.0083774 is OK
- 10.1126/sciadv.abg8562 is OK
- 10.1109/TQE.2021.3116540 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

Wordcount for paper.md is 1463

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@ktahar
Copy link

ktahar commented Feb 22, 2024

I have a few concerns before starting the review.
Substantial part of this project may be the firmware for specific hardware (RFSoC eval board) and I don't have it now.
Situation is similar to the case of pySLM2 #6205.

@phibeck Can I proceed even in this situation? Maybe we must ask the authors to create some mocks to emulate the hardware on PC, in order to check if the software part works as claimed.

@egriendeau Can you possibly include the "modified qick" part in the repository using git submodule (instead of just putting the qick directory)? I would like to see the difference between your modified qick and the original. Having a separate qick repository (with history of original qick) makes this much easier.

@phibeck
Copy link

phibeck commented Feb 27, 2024

Thanks @ktahar for this initial assessment, that's a good point. @egriendeau could you clarify to what extent access to the hardware is needed to assess the quality of the package? I also have trouble opening the README linked on the repo for installation instructions, please check whether this link needs to be updated, thanks.

@ammounce
Copy link

We believe that the package likely stands well on it's own, with out an actual test on hardware:

The main entry point for reviewer or qick-dawg user is the demo jupyter notebook. If we were to make a server for hardware access, the reviewers would run the demo notebook cell by cell and generate output very similar to what is already shown (whlie reading the mark-down cells). Changing parameters on the demo notebook might be do-able by someone with minimal experience with nitrogen vacancy magnetometry/quantum sensing, but changing parameters in the notebook might prove challenging for those with no experience in the field or similar fields.

Nonetheless, we've extensively documented our package. Additionally, we've provided installation, hardware setup instructions, and extensive documentation within our demo jupyter notebook.

We're happy to consider whatever is the best path here.

@egriendeau
Copy link

Additionally, I corrected the link from the main readme to the installation readme

@ktahar
Copy link

ktahar commented Mar 3, 2024

@ammounce Thank you for explanation. I will start my review by playing with the notebook. As I have minimal experience of
diamond NV experiments, I think I can get the feel for it.

@sidihamady
Copy link

sidihamady commented Mar 4, 2024

Review checklist for @sidihamady

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the https://github.com/sandialabs/qick-dawg?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@egriendeau) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • 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.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • 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

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality 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

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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?

@phibeck
Copy link

phibeck commented Mar 4, 2024

Thank you, @egriendeau for fixing the link and @ammounce for clarifying the hardware requirements. Sounds like it should be doable to assess the software via the notebook to a good extent.

Thanks @sidihamady, @14shreyasp and @ktahar for getting the review started. Let me know if you have any questions as you work through your checklists.

@ammounce
Copy link

ammounce commented Mar 4, 2024

@ktahar @sidihamady, @14shreyasp A little more clarification: you do need the hardware to run the demo notebook too.

My comment was that perhaps the demo notebook (as previously) run, could be sufficient. However, if not, let us know and we'll try to setup a test server. I believe this is possible, but could be challenging due to the fact that we're at at National Laboratory and internet security is tight. Nonetheless, I might have a path to making a full test server.

@ktahar
Copy link

ktahar commented Mar 5, 2024

Hi @ammounce , Thank you for clarification. I was misunderstanding that demo notebook can be run without hardware.
But I will try to start the review anyway.

@ktahar
Copy link

ktahar commented Mar 5, 2024

Review checklist for @ktahar

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the https://github.com/sandialabs/qick-dawg?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@egriendeau) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • 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.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • 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

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality 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

Software paper

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • 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?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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?

@phibeck
Copy link

phibeck commented Mar 5, 2024

@ktahar @sidihamady, @14shreyasp A little more clarification: you do need the hardware to run the demo notebook too.

My comment was that perhaps the demo notebook (as previously) run, could be sufficient. However, if not, let us know and we'll try to setup a test server. I believe this is possible, but could be challenging due to the fact that we're at at National Laboratory and internet security is tight. Nonetheless, I might have a path to making a full test server.

Thank you for clarifying @ammounce. According to our policy, the reviewers must be able to test the software. In order to go forward with the review, we do therefore need you to provide a way for the reviewers to run the software, for example via software emulation as suggested by @ktahar. One alternative is to have reviewers with access to the hardware. Perhaps you can let me know if you can think of someone without COI who does have access. Please let me know in which direction you want to proceed. I will place the submission on hold for now until this situation is resolved.

@ktahar @sidihamady, @14shreyasp, thanks again for agreeing to review. Since we do require that reviewers must be able to test the software, this submission is paused for now. I'll keep you updated whether the review is continued at a later point provided there's a way of testing, or whether new reviewers will be assigned who have access to the hardware.

@phibeck phibeck added the paused label Mar 5, 2024
@ktahar
Copy link

ktahar commented Mar 5, 2024

@phibeck I understood the JOSS policy and your decision. Thank you.

@ammounce
Copy link

ammounce commented Mar 6, 2024

@phibeck @ktahar @sidihamady Thanks for the clarification on policy, and apologies for missing that point. I'm away at a conference right now, but in the next few weeks i'll have a definitive answer on the test server and get back to you. It's looking promising though!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants