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

Add operations for setting state and basis state and integrate this with skip_initial_state_prep #955

Merged
merged 110 commits into from
Aug 14, 2024

Conversation

rauletorresc
Copy link
Contributor

@rauletorresc rauletorresc commented Jul 23, 2024

Context: It has been determined that qml.StatePrep takes too long to compile. To facilitate this, it was determined that the optimization of setting the initial state to the operand of qml.StatePrep could help. This is already implemented in lightning.qubit and lightning.kokkos when using them without @qjit.

Description of the Change: This change adds two new JAXPR and MLIR operations. One for qml.StatePrep and another for qml.BasisState. This change includes:

  • Changes in the runtime to add support for this new operations, including changes to the QuantumDevice interface
  • Changes in the TOML specification
  • Changes in the frontend
  • A runtime implementation of qml.BasisState which provides validation at runtime. This is needed because when using jax.numpy.arrays all values are staged.

It was necessary to add new operations instead of reusing quantum.custom op because of the signature of these operations. I.e., quantum.custom expects floating point parameters, but qml.StatePrep expects a vector of complex numbers and qml.BasisState expects an integer index.

Benefits: Possibly faster compile times (has not been verified).

Possible Drawbacks: The introduction of new operations and extension of the runtime for operations that not all devices will need can be seen as a code smell. Kokkos does not implement this since it does not implement the new device API.

Related GitHub Issues: #939

close #939

[sc-69069]

@rauletorresc rauletorresc requested a review from a team July 23, 2024 23:49
@rauletorresc rauletorresc self-assigned this Jul 23, 2024
@erick-xanadu
Copy link
Contributor

@dime10 any comments about the name of the flag in the TOML files being: skip_initial_state_prep?

@paul0403
Copy link
Contributor

@dime10 any comments about the name of the flag in the TOML files being: skip_initial_state_prep?

Maybe do_not_decompose_initial_state_prep? I can see people possibly interpreting skip_initial_state_prep as "ignore this state prep operation altogether"

@dime10
Copy link
Contributor

dime10 commented Jul 30, 2024

@dime10 any comments about the name of the flag in the TOML files being: skip_initial_state_prep?

Yes, skip makes no sense to me in the context of the TOML file. Looking at the existing flags that indicate "support" for something:

        # If the device supports mid circuit measurements natively
        mid_circuit_measurement = true
        # This field is currently unchecked but it is reserved for the purpose of
        # determining if the device supports dynamic qubit allocation/deallocation.
        dynamic_qubit_management = false

I would say just initial_state_prep is fine.

@erick-xanadu erick-xanadu force-pushed the raultorres/stateprep_lightning branch from 49b70cf to 47ed503 Compare July 31, 2024 17:57
@erick-xanadu erick-xanadu changed the title StatePrep does not decompose for Lightning [frontend/middle-end/runtime] Add operations for setting state and basis state and integrate this with skip_initial_state_prep Jul 31, 2024
@erick-xanadu erick-xanadu changed the title [frontend/middle-end/runtime] Add operations for setting state and basis state and integrate this with skip_initial_state_prep Add operations for setting state and basis state and integrate this with skip_initial_state_prep Jul 31, 2024
@erick-xanadu erick-xanadu force-pushed the raultorres/stateprep_lightning branch from 5b30977 to 93e195c Compare July 31, 2024 23:44
@erick-xanadu erick-xanadu marked this pull request as ready for review July 31, 2024 23:46
@erick-xanadu erick-xanadu force-pushed the raultorres/stateprep_lightning branch 2 times, most recently from 1b8c01f to 449dc68 Compare August 2, 2024 12:43
erick-xanadu added a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Aug 2, 2024
**Context:** We would like Catalyst to optimize the first instance of
`qml.StatePrep` by just setting the state vector in PennyLane lightning.

**Description of the Change:** To achieve this, we set a flag in the
TOML files where it denotes whether the value for the flag
`skip_initial_state_prep` used during decomposition. This flag is
already in PennyLane and will skip the initial state preparation.

There are other ways to achieve this but this is the least invasive way
to do so. Some other alternatives that were discussed:

1. Adding StatePrep to the list of supported operations: this is not a
good idea as it would lead StatePrep to not be decomposed ever.
2. Creating a new operation and map StatePrep directly into this new
operation only in the context of Catalyst. This would lead to setting
this new `qml` Operation in the list of supported operations on the
device, even though it would never be seen except when using with
Catalyst.

The way this was achieved was not to create a new `qml` Operation, only
an JAXPR/MLIR operation and bind `StatePrep` to this JAXPR operation and
lower it through MLIR. So, no new `qml` operations.

**Benefits:** Optimization

**Possible Drawbacks:** More flags. No one likes flags.

**Related GitHub Issues:** Will be followed by this
PennyLaneAI/catalyst#955 pull request in
Catalyst.

[sc-69069]

---------

Co-authored-by: ringo-but-quantum <[email protected]>
Co-authored-by: Ali Asadi <[email protected]>
@erick-xanadu erick-xanadu force-pushed the raultorres/stateprep_lightning branch from 398d9e0 to e4928a7 Compare August 2, 2024 20:19
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.89%. Comparing base (4f18276) to head (507302c).
Report is 212 commits behind head on main.

Files with missing lines Patch % Lines
runtime/include/QuantumDevice.hpp 0.00% 4 Missing ⚠️
frontend/catalyst/jax_tracer.py 93.93% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #955      +/-   ##
==========================================
- Coverage   97.92%   97.89%   -0.04%     
==========================================
  Files          75       75              
  Lines       10561    10684     +123     
  Branches     1218     1226       +8     
==========================================
+ Hits        10342    10459     +117     
- Misses        172      177       +5     
- Partials       47       48       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@erick-xanadu
Copy link
Contributor

@dime10 you mentioned that we shouldn't support contextual decomposition. (I.e., if qml.StatePrep is inside grad, it is okay to emit an error). There is a demo with this. I added a small hack.

@dime10
Copy link
Contributor

dime10 commented Aug 2, 2024

@dime10 you mentioned that we shouldn't support contextual decomposition. (I.e., if qml.StatePrep is inside grad, it is okay to emit an error). There is a demo with this. I added a small hack.

I just mentioned that we don't do this for any other operations yet, so it's not a requirement in order to add the support for this op. If you implemented it anyways that's fine 👍
Which demo?

(currently reviewing)

@erick-xanadu
Copy link
Contributor

@dime10 adaptive circuits demo. I just added qml.RX(0, wires=[0]) before BasisState. Not a great hack. 😅

@paul0403
Copy link
Contributor

paul0403 commented Aug 3, 2024

Reminder to update the same changes on lightning kokkos to the lightning repo (e.g. PennyLaneAI/pennylane-lightning#819)

multiphaseCFD pushed a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Aug 5, 2024
**Context:** We would like Catalyst to optimize the first instance of
`qml.StatePrep` by just setting the state vector in PennyLane lightning.

**Description of the Change:** To achieve this, we set a flag in the
TOML files where it denotes whether the value for the flag
`skip_initial_state_prep` used during decomposition. This flag is
already in PennyLane and will skip the initial state preparation.

There are other ways to achieve this but this is the least invasive way
to do so. Some other alternatives that were discussed:

1. Adding StatePrep to the list of supported operations: this is not a
good idea as it would lead StatePrep to not be decomposed ever.
2. Creating a new operation and map StatePrep directly into this new
operation only in the context of Catalyst. This would lead to setting
this new `qml` Operation in the list of supported operations on the
device, even though it would never be seen except when using with
Catalyst.

The way this was achieved was not to create a new `qml` Operation, only
an JAXPR/MLIR operation and bind `StatePrep` to this JAXPR operation and
lower it through MLIR. So, no new `qml` operations.

**Benefits:** Optimization

**Possible Drawbacks:** More flags. No one likes flags.

**Related GitHub Issues:** Will be followed by this
PennyLaneAI/catalyst#955 pull request in
Catalyst.

[sc-69069]

---------

Co-authored-by: ringo-but-quantum <[email protected]>
Co-authored-by: Ali Asadi <[email protected]>
@erick-xanadu
Copy link
Contributor

@paul0403 changes will be on this PR: PennyLaneAI/pennylane-lightning#838 Thanks!

@erick-xanadu erick-xanadu added the author:build-wheels Run the wheel building workflows on this Pull Request label Aug 5, 2024
erick-xanadu and others added 3 commits August 13, 2024 15:40
@erick-xanadu erick-xanadu requested a review from dime10 August 13, 2024 19:43
@erick-xanadu erick-xanadu added the author:build-wheels Run the wheel building workflows on this Pull Request label Aug 13, 2024
doc/changelog.md Outdated Show resolved Hide resolved
@erick-xanadu
Copy link
Contributor

I added building wheels as I want to test macOS specifically for the use of int8_t for the basis state.

Co-authored-by: David Ittah <[email protected]>
@dime10
Copy link
Contributor

dime10 commented Aug 13, 2024

I added building wheels as I want to test macOS specifically for the use of int8_t for the basis state.

Sounds good, I'm trying to test locally right now but the lightning wheels weren't available, I have notified the perf team though.

@dime10
Copy link
Contributor

dime10 commented Aug 13, 2024

I added building wheels as I want to test macOS specifically for the use of int8_t for the basis state.

Sounds good, I'm trying to test locally right now but the lightning wheels weren't available, I have notified the perf team though.

Alright they should all be available now 👍

Copy link
Contributor

@dime10 dime10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@erick-xanadu erick-xanadu removed the author:build-wheels Run the wheel building workflows on this Pull Request label Aug 13, 2024
@erick-xanadu
Copy link
Contributor

Do note that coverage is wrong here, coverage is not calculated with OpenQASM3 runtime.

@erick-xanadu erick-xanadu merged commit dcbc683 into main Aug 14, 2024
43 of 45 checks passed
@erick-xanadu erick-xanadu deleted the raultorres/stateprep_lightning branch August 14, 2024 15:50
erick-xanadu added a commit that referenced this pull request Aug 15, 2024
**Context:** #955 could not test lightning.kokkos due to the changes in
the QuantumDevice interface along with the separation of repositories.
It can only be tested once lightning.kokkos has been built and released
with the changes in the QuantumDevice interface.

**Description of the Change:** Restore kokkos tests

EDIT: Do not merge until there's a new dev release from lightning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] qml.StatePrep decomposes unnecessarily for Lightning, and the decomposition is very slow
4 participants