openjournals / joss-reviews

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

[REVIEW]: Catalyst: a Python JIT compiler for auto-differentiable hybrid quantum programs #6720

Closed editorialbot closed 4 months ago

editorialbot commented 7 months ago

Submitting author: !--author-handle-->@josh146<!--end-author-handle-- (Joshua Izaac) Repository: https://github.com/PennyLaneAI/catalyst Branch with paper.md (empty if default branch): joss-paper Version: v0.7.0 Editor: !--editor-->@danielskatz<!--end-editor-- Reviewers: @pmcao, @otbrown Archive: 10.5281/zenodo.12696447

Status

status

Status badge code:

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

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

@pmcao & @otbrown, 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 @danielskatz 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 @otbrown

πŸ“ Checklist for @pmcao

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.90  T=0.27 s (1218.3 files/s, 238771.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          92           5928           4939          15988
C++                             69           3456           1755          13053
C/C++ Header                    51            949           1393           3133
reStructuredText                19           1171           1522           1826
YAML                            20            227             40           1556
Markdown                         8            494              0           1392
CMake                           38            152             46            674
Jupyter Notebook                 4              0           2203            598
make                             5             74             11            379
TeX                              2             83              4            323
LLVM IR                          3             93             64            177
JSON                             3              0              0            118
SVG                              6              0              0             85
Dockerfile                       2             18             11             38
TOML                             1              6              0             34
Bourne Shell                     2              9              0             30
HTML                             1              6              0             27
INI                              1              0              0              3
-------------------------------------------------------------------------------
SUM:                           327          12666          11988          39434
-------------------------------------------------------------------------------

Commit count by author:

    66  Ali Asadi
    59  erick-xanadu
    54  David Ittah
    19  Sergei Mironov
    17  Josh Izaac
    14  Jacob Mai Peng
    13  Romain Moyard
     1  Paul Finlay
     1  dependabot[bot]
editorialbot commented 7 months ago

Paper file info:

πŸ“„ Wordcount for paper.md is 1395

βœ… The paper includes a Statement of need section

editorialbot commented 7 months ago

License info:

βœ… License found: Apache License 2.0 (Valid open source OSI approved license)

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

OK DOIs

- 10.5281/zenodo.6385575 is OK
- 10.1088/1367-2630/18/2/023023 is OK
- 10.1103/physreva.104.052402 is OK
- 10.1109/CGO51591.2021.9370308 is OK
- 10.22331/q-2022-03-30-677 is OK
- 10.1103/physreva.99.032331 is OK

MISSING DOIs

- No DOI given, and none found for title: IBM Quantum Experience
- 10.1201/9781003374404-3 may be a valid DOI for title: Qiskit
- No DOI given, and none found for title: Cirq
- No DOI given, and none found for title: Amazon Braket
- No DOI given, and none found for title: Pennylane: Automatic differentiation of hybrid qua...
- No DOI given, and none found for title: A quantum approximate optimization algorithm
- No DOI given, and none found for title: JAX: composable transformations of Python+NumPy pr...
- No DOI given, and none found for title: PennyLane Lightning: fast state-vector simulators ...
- No DOI given, and none found for title: LLVM: A Compilation Framework for Lifelong Program...
- No DOI given, and none found for title: Quantum computational advantage with a programmabl...
- No DOI given, and none found for title:  Instead of Rewriting Foreign Code for Machine Lea...
- No DOI given, and none found for title: QIR Specification
- No DOI given, and none found for title: AutoGraph: Imperative-style Coding with Graph-base...
- No DOI given, and none found for title: Efficient and modular implicit differentiation
- No DOI given, and none found for title: PennyLane Demos

INVALID DOIs

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

otbrown commented 7 months ago

Review checklist for @otbrown

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

danielskatz commented 7 months ago

@pmcao and @otbrown - Thanks for agreeing to review this submission. This is the review thread for the paper. All of our communications will happen here from now on.

As you can see above, you each should use the command @editorialbot generate my checklist to create your review checklist. @editorialbot commands need to be the first thing in a new comment.

As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#6720 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if either of you require some more time. We can also use editorialbot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@danielskatz) if you have any questions/concerns.

danielskatz commented 6 months ago

πŸ‘‹ @pmcao - I know it's just been two weeks at this point, but it would be great if you could use the command @editorialbot generate my checklist to create your review checklist. @editorialbot commands need to be the first thing in a new comment.

Once you do this, you should be able to check off the first two items fairly quickly.

danielskatz commented 6 months ago

πŸ‘‹ @otbrown - How is your review coming along? Is anything blocking you?

pmcao commented 6 months ago

@editorialbot generate my checklist

danielskatz commented 6 months ago

@pmcao - can you try that again? I'm not sure why it didn't work immediately...

otbrown commented 6 months ago

πŸ‘‹ @otbrown - How is your review coming along? Is anything blocking you?

All good so far! Only blocker is me sitting at my desk. Got some time set aside this afternoon to try out the remaining code examples.

otbrown commented 6 months ago

@danielskatz I have some comments, but they all relate to this specific write-up, rather than the software itself. Should I still create an issue in the target repository, or should it just be raised here?

danielskatz commented 6 months ago

@otbrown - sorry for missing this. You can do this either way; it's up to you.

danielskatz commented 6 months ago

@pmcao - can you try the @editorialbot generate my checklist again? I'm not sure why it didn't work immediately...

otbrown commented 6 months ago

Hi @josh146,

Sorry you've been waiting so long for this review! Catalyst itself looks great, but as above, I have some minor comments relating to the write-up for JOSS. I've ordered my comments from most to least trivial, but I don't think any of them entail significant work to address. Please let me know if you have any questions.

danielskatz commented 5 months ago

πŸ‘‹ @josh146 - Please see the comments above and let us know how you will respond to them

danielskatz commented 5 months ago

@pmcao - can you try the @editorialbot generate my checklist again? I'm not sure why it didn't work immediately...

pmcao commented 5 months ago

Review checklist for @pmcao

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

josh146 commented 5 months ago

Thanks @danielskatz @otbrown! I've implemented all code review recommendations here: https://github.com/PennyLaneAI/catalyst/pull/807. Will comment again as soon as it is merged πŸ™‚

I've also directly updated the paper for the following two bullet points:

  • A space is missing before the citation on line 49.
  • The reference on line 131 (Lattner & Adve 2004) lists the conference as 'Cgo' rather than 'International Symposium on Code Generation and Optimization (CGO)' as Lattner et al. 2021.
danielskatz commented 5 months ago

πŸ‘‹ @otbrown - does this satisfy your concerns? Are there more?

danielskatz commented 5 months ago

πŸ‘‹ @pmcao - How is your review coming along? Is there anything blocking you?

pmcao commented 5 months ago

Checking on functionalities.

I will add a more detailed written review over the weekend.

josh146 commented 5 months ago

The changes as suggested by @otbrown have now been merged in to the paper :)

danielskatz commented 5 months ago

πŸ‘‹ @otbrown - can you now check off your final item?

danielskatz commented 5 months ago

πŸ‘‹ @pmcao - just checking on your progress again... Is anything holding you up that I or the author can help with?

otbrown commented 5 months ago

Hi all,

Apologies for another delay -- I was on a short holiday in London this week, I can highly recommend the stage adaptation of Miyazaki's Spirited Away ;)

Thanks for the quick response @josh146. Changes look good to me, and I've completed my checklist!

danielskatz commented 5 months ago

πŸ‘‹ @pmcao - just checking on your progress again... Is anything holding you up that I or the author can help with?

pmcao commented 5 months ago

@josh146

This is a great effort to bridge classical and quantum runtime. Here are the comments:

Write up:

Misc:

I have a few minor comments that I'd like to add before the end of this week. Hope those help with the documentation clarity and adoption of Catalyst!

I also created an issue in the joss paper branch: https://github.com/PennyLaneAI/catalyst/issues/874

josh146 commented 5 months ago

Hi @pmcao, thanks for your review!

I am curious how Catalyst handles faults in quantum hardware? If there are examples of faults and the code like exception handling that'd be great. Since the paper claims "advanced quantum programming features essential for fault-tolerant hardware support" more descriptions in fault handling is needed.

There is a slight subtlety in this phrase here I think. With the Catalyst framework, we are more focused (at the moment) on enabling research that is focused on exploring, building, analyzing/executing algorithms designed for fault-tolerant quantum hardware. That is, the programming approach and features you need for algorithms targeting fault-tolerant quantum hardware is different to the features you need for targeting near-term quantum hardware!

What we don't currently support in Catalyst is implementing direct support for error correction (that is, compiling the algorithm down to physical hardware using error correction schemes that would correct against hardware faults). This is a very active field, and something we are likely to explore in the future :)

The Catalyst documentation seems extensive. There are four examples, however, I couldn't get the taxonomy of how those examples are related together.

I can briefly run through some of these examples:

So the examples aren't thematically related, apart from being different (and common!) workflows you see across different research areas in quantum computing.

Coming from signal processing background, can you add an example of an HMM implemented in Catalyst with quantum hardware, since HMMs are widely used for inference in many of the domains.

Unfortunately, myself and the team are not familiar with HMMs (or their quantum algorithm equivalents), so likely would not be able to provide this example!

danielskatz commented 5 months ago

@pmcao - can you respond to @josh146 ☝️ ?

Also, you previously mentioned

I have a few minor comments that I'd like to add before the end of this week. Hope those help with the documentation clarity and adoption of Catalyst!

Has this happened?

pmcao commented 5 months ago

I referred to this comment in this issue: https://github.com/PennyLaneAI/catalyst/issues/887

Copying it over here to respond to author's comments:

Comment:

On balance, the paper has motivated the need, described three critical pillars of the JIT compiler infrastructure, and made significant open-source contributions.

My takeaway from reviewing this paper is that fault-tolerant quantum computing has to be supported end-to-end in both hardware, mid-circuit, and classical processing because the fault can manifest at any part of the pipeline, for example, Silent Data Corruption in the classical processing part. As the author mentioned, "becoming clear that we cannot separate classical and quantum parts of the program".

This JOSS paper would benefit from a Discussion / Future Work in which the authors, with their expertise, could point to future directions in this area.

pmcao commented 5 months ago

This minor issue would need a few sentences to be resolved.

After that, the submission can be published.

This comment finalizes my review.

danielskatz commented 5 months ago

πŸ‘‹ @josh146 - please let us know when you've worked on this above from @pmcao - I think we'll basically be done then.

josh146 commented 5 months ago

Hi @pmcao, thank you for your review comments!

This JOSS paper would benefit from a Discussion / Future Work in which the authors, with their expertise, could point to future directions in this area.

I have added a new section to the manuscript, detailing future directions for the catalyst hybrid compilation stack:

# Discussion and future work

The Catalyst hybrid compilation stack as presented here provides an end-to-end infrastructure
to explore next-generation dynamic hybrid quantum-classical algorithms, by allowing for 
workflows that support compressed representation of large, highly structured quantum 
algorithms, as well as mid-circuit measurements with arbitrary classical processing and 
feedforward.

The Catalyst software stack will continue to be developed alongside research, algorith, and 
hardware needs, with potential future work including support for quantum hardware control 
systems, building out a library of MLIR quantum compilation passes for optimizing quantum 
circuits (without unrolling classical control structure), and explorations of dynamic quantum 
error mitigation and proof-of-concept error correction experiments.

Quantum software is driving many new results and ideas in quantum computing research, and the 
PennyLane framework has already been used in a number of scientific publications 
[@delgado2021variational; @wierichs2021general] and educational materials [@demos]. By enabling 
researchers to scale up their ideas and algorithms, and execute on both near-term and future 
quantum hardware, the software presented here will help drive future research in quantum 
computing.
danielskatz commented 5 months ago

@josh146 - somehow, the discussion of https://github.com/PennyLaneAI/catalyst/issues/887 seems to have moved into this issue rather than that one. Can you make your comment there, and if you think that issue is resolved, close it. Then you can use the command @editorialbot generate pdf here to make a new, up-to-date PDF. (@editorialbot commands need to the first thing in a comment.)

josh146 commented 5 months ago

@editorialbot generate pdf

Thanks @danielskatz, I've closed the issue.

editorialbot commented 5 months ago

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

danielskatz commented 5 months ago

At this point could you:

At the same time, I will proofread the submission and might suggest changes (noting that the changes will not need to be included in the archived version, where we focus on the code, since we will also publish the paper itself built from the final source)

danielskatz commented 5 months ago

@josh146 - My proofreading suggestions are in https://github.com/PennyLaneAI/catalyst/pull/888 - please merge this, or let me know what you disagree with.

josh146 commented 5 months ago

Thanks @danielskatz!

Make a tagged release of your software, and list the version tag of the archived version here.

Do you mean a GitHub release, or simply a git tag? A github release is non-trivial at the moment, as we have a lot of continuous integration that would be triggered.

We have a new release of Catalyst (0.7.0) scheduled on July 10th, so another option is to wait for then.

danielskatz commented 5 months ago

I meant a tag to the state of the software repo at the time that the review has finished, which could just be a tag, or could be a release. The reason for a release might be to use it to automatically make a deposit on Zenodo using the GitHub - Zenodo linkage for this, but you can make the archive however you want, including manually. Also, waiting until July 10 would be ok too.

danielskatz commented 5 months ago

@editorialbot check references

danielskatz commented 5 months ago

@editorialbot generate pdf

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

OK DOIs

- 10.5281/zenodo.6385575 is OK
- 10.1088/1367-2630/18/2/023023 is OK
- 10.48550/arXiv.1411.4028 is OK
- 10.1103/physreva.104.052402 is OK
- 10.1109/CGO.2004.1281665 is OK
- 10.1109/CGO51591.2021.9370308 is OK
- 10.1038/s41586-022-04725-x is OK
- 10.22331/q-2022-03-30-677 is OK
- 10.1103/physreva.99.032331 is OK

MISSING DOIs

- No DOI given, and none found for title: IBM Quantum Experience
- 10.1201/9781003374404-3 may be a valid DOI for title: Qiskit
- No DOI given, and none found for title: Cirq
- No DOI given, and none found for title: Amazon Braket
- No DOI given, and none found for title: Pennylane: Automatic differentiation of hybrid qua...
- No DOI given, and none found for title: JAX: composable transformations of Python+NumPy pr...
- No DOI given, and none found for title: PennyLane Lightning: fast state-vector simulators ...
- No DOI given, and none found for title:  Instead of Rewriting Foreign Code for Machine Lea...
- No DOI given, and none found for title: QIR Specification
- No DOI given, and none found for title: Efficient and modular implicit differentiation
- No DOI given, and none found for title: PennyLane Demos

INVALID DOIs

- https://doi.org/10.48550/arXiv.1810.08061 is INVALID because of 'https://doi.org/' prefix
danielskatz commented 5 months ago

@josh146 can you also fix the doi entry I made incorrectly, with the extra prefix?

editorialbot commented 5 months ago

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

danielskatz commented 5 months ago

Actually, I went ahead and fixed the DOI in https://github.com/PennyLaneAI/catalyst/pull/889, as I saw another one to add too.

josh146 commented 5 months ago

Done!