Adds Catalyst specification files. #566

erick-xanadu · 2023-11-24T19:41:46Z

Context: The following files improve the compatibility between pennylane-lightning and catalyst. The initial motivation behind these files was a way to distinguish gates which are allowed in pennylane-lightning's of qml.Device but are implemented in the C/C++ library via applyMatrix. Without these applyMatrix gates, catalyst had to rely on decomposition rules which yielded an initially unexpected (but not unsurprising behaviour). For example, OrbitalRotation would be decomposed into more than 50 gates as opposed to being applied as a matrix. With these files, catalyst can now specify custom decomposition rules for gates which are applied via applyMatrix and match pennylane-lightning's behaviour more closely. Please see ADR 73 for the details.

Description of the Change: Adds initial configuration files for lightning.qubit and lightning.kokkos.

Benefits: Better compatibility between pennylane-lightning and catalyst.

Related GitHub Issues: PennyLaneAI/catalyst#369

[sc-46426]

codecov · 2023-11-24T19:48:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8770a96) 98.69% compared to head (c2af0b1) 98.69%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #566   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files         167      167           
  Lines       23813    23819    +6     
=======================================
+ Hits        23502    23508    +6     
  Misses        311      311

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

pennylane_lightning/core/__init__.py

pennylane_lightning/core/src/lightning.kokkos.toml

pennylane_lightning/core/src/lightning.qubit.toml

pennylane_lightning/core/src/lightning.kokkos.toml

pennylane_lightning/core/src/lightning.qubit.toml

MANIFEST.in

AmintorDusko

Nice job so far! I left some comments.

pennylane_lightning/core/src/lightning.qubit.toml

.github/CHANGELOG.md

pennylane_lightning/core/__init__.py

vincentmr

Thanks for this @erick-xanadu, I've left a few suggestions.

pennylane_lightning/core/src/lightning.kokkos.toml

pennylane_lightning/core/src/lightning.qubit.toml

pennylane_lightning/core/src/lightning.kokkos.toml

MANIFEST.in

pennylane_lightning/core/src/lightning.kokkos.toml

maliasadi

Thank you @erick-xanadu! I have a few minor comments, but it looks good to me.

MANIFEST.in

pennylane_lightning/core/src/lightning.kokkos.toml

.github/CHANGELOG.md

AmintorDusko

Looks good to me!

pennylane_lightning/core/src/lightning_qubit.toml

vincentmr

Small comment, but otherwise looks good to me.

MANIFEST.in

maliasadi

Thanks @erick-xanadu! 🎉

dime10

Looks great 👍

pennylane_lightning/core/src/lightning_gpu.toml

pennylane_lightning/lightning_kokkos/lightning_kokkos.py

pennylane_lightning/core/src/lightning_gpu.toml

pennylane_lightning/core/src/lightning_kokkos.toml

pennylane_lightning/core/src/lightning_qubit.toml

vincentmr

Still looks good to me, but I left a few comments.

pennylane_lightning/core/src/lightning_gpu.toml

pennylane_lightning/core/src/lightning_qubit.toml

AmintorDusko · 2023-12-12T16:00:44Z

Hi @erick-xanadu, please merge the master in your branch one more time, and also update the _version.py file. So far, _version_ = "0.34.0-dev17" needs to be updated to _version_ = "0.34.0-dev18"

dime10 · 2023-12-12T16:25:26Z

I have a question for the team, among the gates that the lightning device lists as "supported" (that is PL will not currently decompose them, full list here) but doesn't have native kernel support for (should correspond to this list), which gates would you rather see decomposed via op.decomposition() vs implemented as a QubitUnitary?

Generally this choice will be up to the device developer in our framework, although the compiler may choose to override that preference if needed. The preference is expressed in the TOML file via the matrix and decomp fields. Note that the contents of these fields need only cover the gates the device considers "supported" in its PennyLane execution pipeline (first link).

I think it comes down to the following operations:

        "BasisState",
        "ECR",
        "QubitStateVector",
        "StatePrep",
        "ControlledQubitUnitary",
        "DiagonalQubitUnitary",
        "SX",
        "ISWAP",
        "PSWAP",
        "SISWAP",
        "SQISW",
        "CPhase",
        "QubitCarry",
        "QubitSum",
        "OrbitalRotation",
        "QFT",
        "MultiControlledX",

The kind of decisions that could be made for example is "StatePrep would generate a huge matrix, maybe decomposition is better here" and "ISWAP is small gate, it is best implement as a matrix", but we'll leave the partition up to you.

vincentmr · 2023-12-12T18:30:23Z

For a start

# Matrix 
"ECR",
"SX",
"ISWAP",
"PSWAP",
"SISWAP",
"SQISW",
"CPhase",
"OrbitalRotation",
"QubitCarry",
"QubitSum",
"DiagonalQubitUnitary",
# Matrix if not too many controls, but ideally use controlled_wires argument
"ControlledQubitUnitary",
"MultiControlledX",
# Decomposition        
"BasisState",
"QubitStateVector",
"StatePrep",
"QFT",

That said, decomposition is sometimes necessary to transform multi-parameter gates into single-parameter gates for the adjoint method. I would also add we could quickly add kernels for most of these to get better performance, i.e. avoid decomposition and useless flops.

dime10 · 2023-12-12T18:38:55Z

Thank you @vincentmr, appreciate the list, let's go with that for now :)

I would also add we could quickly add kernels for most of these to get better performance, i.e. avoid decomposition and useless flops.

We are happy to update our pipeline whenever new kernels become available!

That said, decomposition is sometimes necessary to transform multi-parameter gates into single-parameter gates for the adjoint method.

That's true we have considered this as well, we are going to force decomposition if required by differentiation (hence the device spec is only a preference).

erick-xanadu · 2023-12-12T18:45:18Z

@vincentmr I will be updating the files to the following preference then:

# Matrix 
"ECR",
"SX",
"ISWAP",
"PSWAP",
"SISWAP",
"SQISW",
"CPhase",
"OrbitalRotation",
"QubitCarry",
"QubitSum",
"DiagonalQubitUnitary",
"MultiControlledX",

# Decomposition        
"BasisState",
"QubitStateVector",
"StatePrep",
"QFT",

# Native
"ControlledQubitUnitary",

EDIT: Moved MultiControlledX to matrix

erick-xanadu · 2023-12-12T20:23:37Z

@vincentmr @AmintorDusko the PR appears to be in the final state. At least according to the discussion that I have followed.

What I was told by @dime10 and from the discussions

We are removing Adjoint gates from all three lists.
MultiControlledX is a matrix gate
I am following @vincentmr gate decomposition strategy
Version is _version_ = "0.34.0-dev18"
The files have been moved to the where the device class exists.

vincentmr · 2023-12-12T20:40:10Z

I am following @vincentmr gate decomposition strategy

On second thought (sorry ...), due to the lack of support for control values and because their matrix representation can grow large with many controls, I think it would be best to put ControlledQubitUnitary and MultiControlledX in decomp in all devices for now.

erick-xanadu · 2023-12-12T20:51:04Z

Not a problem @vincentmr, happy to make the change: 159f6a8

dime10 · 2023-12-12T21:12:23Z

@vincentmr @erick-xanadu Sorry to come back to this, but from the information you provided above I think ControlledQubitUnitary is actually natively supported by lightning, it just needs the be mapped to the right C++ function (apply matrix with control wires). We can add support for it in Catalst in the near future.
(Decomposition would also pose a problem in many instances because PennyLane can only decompose 2-qubit unitaries.)

mlxd · 2023-12-12T21:17:17Z

Hi all, I'm manually overriding the merge of this onto master to overcome the dev version script CI failure.

vincentmr · 2023-12-12T21:17:55Z

@vincentmr @erick-xanadu Sorry to come back to this, but from the information you provided above I think ControlledQubitUnitary is actually natively supported by lightning, it just needs the be mapped to the right C++ function (apply matrix with control wires). We can add support for it in Catalst in the near future. (Decomposition would also pose a problem in many instances because PennyLane can only decompose 2-qubit unitaries.)

It is natively supported if all control values are 1. If the assumption somehow hold true, than I'd say list it in native. Otherwise, the best would be to decomp into ControlledQubitUnitary and apply PauliX before and after on the wires which control to 0. Is that possible and would that then qualify as decomp or native?

erick-xanadu · 2023-12-12T21:21:34Z

@dime10 and @vincentmr I don't think we should worry too much about where gates live for the time being. A lot of behaviour is overriden by Catalyst and this includes ControlledQubitUnitary. With the current behaviour, if ControlledQubitUnitary inherits from qml.ops.Controlled then it will be converted to qml.QubitUnitary if it doesn't then it will be attempted to decompose and fail if it can't be decomposed.

I think this is a bit too future thinking and there is no reason why we can't move this gate from one list to the other in the future as well. Happy to make any changes though.

dime10 · 2023-12-12T21:23:58Z

It is natively supported if all control values are 1. If the assumption somehow hold true, than I'd say list it in native. Otherwise, the best would be to decomp into ControlledQubitUnitary and apply PauliX before and after on the wires which control to 0. Is that possible and would that then qualify as decomp or native?

You're right, I didn't realize ControlledQubitUnitary also had control values 🤦 In that case I agree with you, except that matrix might be a better choice because of the issue I mentioned about the number of qubits (see docs).

vincentmr · 2023-12-12T21:25:21Z

@erick-xanadu and @dime10 I think you know better than me which list it should go in at this point. I wanted to point out limitations in the current implementation and if there are any doubt on your side feel free to ask. On that I'll let you decide and stamp this PR with approval. Cheers.

erick-xanadu · 2023-12-12T21:30:11Z

Ok @dime10 said to move to matrix, so that is the change I implemented c2af0b1

erick-xanadu marked this pull request as draft November 24, 2023 19:44

erick-xanadu force-pushed the eochoa/2023-11-24/catalyst-spec-files branch 4 times, most recently from f28a489 to bb5e941 Compare November 24, 2023 21:43

erick-xanadu mentioned this pull request Nov 24, 2023

[Frontend][Runtime] lightning.qubit specifies its supported capabilities, compilation options, and runtime options in a config file PennyLaneAI/catalyst#369

Merged

21 tasks

erick-xanadu marked this pull request as ready for review November 24, 2023 22:04

maliasadi reviewed Nov 28, 2023

View reviewed changes

erick-xanadu requested a review from maliasadi November 30, 2023 16:09

mlxd requested a review from a team November 30, 2023 16:36

AmintorDusko reviewed Nov 30, 2023

View reviewed changes

pennylane_lightning/core/src/lightning.qubit.toml Outdated Show resolved Hide resolved

.github/CHANGELOG.md Outdated Show resolved Hide resolved

pennylane_lightning/core/__init__.py Outdated Show resolved Hide resolved

vincentmr reviewed Nov 30, 2023

View reviewed changes

maliasadi reviewed Dec 1, 2023

View reviewed changes

MANIFEST.in Outdated Show resolved Hide resolved

pennylane_lightning/core/src/lightning.kokkos.toml Outdated Show resolved Hide resolved

pennylane_lightning/core/src/lightning.kokkos.toml Outdated Show resolved Hide resolved

trbromley reviewed Dec 4, 2023

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

erick-xanadu force-pushed the eochoa/2023-11-24/catalyst-spec-files branch from 1bb8d0c to a3d2f6f Compare December 4, 2023 16:48

erick-xanadu requested review from AmintorDusko, maliasadi and vincentmr December 4, 2023 17:51

AmintorDusko approved these changes Dec 4, 2023

View reviewed changes

erick-xanadu force-pushed the eochoa/2023-11-24/catalyst-spec-files branch from 3dbefaa to 0a4dfce Compare December 4, 2023 18:25

dime10 reviewed Dec 4, 2023

View reviewed changes

pennylane_lightning/core/src/lightning_qubit.toml Outdated Show resolved Hide resolved

dime10 reviewed Dec 4, 2023

View reviewed changes

pennylane_lightning/core/src/lightning_qubit.toml Outdated Show resolved Hide resolved

vincentmr approved these changes Dec 4, 2023

View reviewed changes

MANIFEST.in Outdated Show resolved Hide resolved

maliasadi approved these changes Dec 5, 2023

View reviewed changes

erick-xanadu requested a review from dime10 December 5, 2023 23:03

dime10 approved these changes Dec 6, 2023

View reviewed changes

pennylane_lightning/core/src/lightning_gpu.toml Outdated Show resolved Hide resolved

pennylane_lightning/core/src/lightning_gpu.toml Outdated Show resolved Hide resolved

pennylane_lightning/lightning_kokkos/lightning_kokkos.py Outdated Show resolved Hide resolved

dime10 reviewed Dec 6, 2023

View reviewed changes

pennylane_lightning/core/src/lightning_gpu.toml Outdated Show resolved Hide resolved

maliasadi requested changes Dec 6, 2023

View reviewed changes

maliasadi approved these changes Dec 6, 2023

View reviewed changes

Update toml files.

224eb0b

vincentmr reviewed Dec 12, 2023

View reviewed changes

Merge branch 'master' into eochoa/2023-12-08/spec

2a7ccfe

erick-xanadu added 2 commits December 12, 2023 11:12

Merge branch 'master' into eochoa/2023-12-08/spec

651e3fc

Update dev version.

8c40df2

erick-xanadu added 4 commits December 12, 2023 13:59

Update toml according to maintainers preference.

7fea774

Move adjoint to native gates.

d50bcb9

Move files.

4df3f84

Remove adjoint gates from toml.

3d9fcf5

Move ControlledQubitUnitary and MultiControlledX to decomp.

159f6a8

erick-xanadu requested a review from mlxd December 12, 2023 21:12

Move ControlledQubitUnitary to matrix gate set.

c2af0b1

mlxd approved these changes Dec 12, 2023

View reviewed changes

mlxd merged commit 998362a into PennyLaneAI:master Dec 12, 2023
80 of 81 checks passed

dime10 mentioned this pull request Jan 12, 2024

[QIR] MultiControlledX for lightning devices. PennyLaneAI/catalyst#380

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Catalyst specification files. #566

Adds Catalyst specification files. #566

erick-xanadu commented Nov 24, 2023 •

edited by dime10

Loading

codecov bot commented Nov 24, 2023 •

edited

Loading

AmintorDusko left a comment

vincentmr left a comment

maliasadi left a comment

AmintorDusko left a comment

vincentmr left a comment

maliasadi left a comment

dime10 left a comment

vincentmr left a comment

AmintorDusko commented Dec 12, 2023

dime10 commented Dec 12, 2023

vincentmr commented Dec 12, 2023

dime10 commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023 •

edited

Loading

erick-xanadu commented Dec 12, 2023

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023

dime10 commented Dec 12, 2023

mlxd commented Dec 12, 2023

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023 •

edited

Loading

dime10 commented Dec 12, 2023 •

edited

Loading

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023

Adds Catalyst specification files. #566

Adds Catalyst specification files. #566

Conversation

erick-xanadu commented Nov 24, 2023 • edited by dime10 Loading

codecov bot commented Nov 24, 2023 • edited Loading

Codecov Report

AmintorDusko left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

maliasadi left a comment

Choose a reason for hiding this comment

AmintorDusko left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

maliasadi left a comment

Choose a reason for hiding this comment

dime10 left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

AmintorDusko commented Dec 12, 2023

dime10 commented Dec 12, 2023

vincentmr commented Dec 12, 2023

dime10 commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023 • edited Loading

erick-xanadu commented Dec 12, 2023

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023

dime10 commented Dec 12, 2023

mlxd commented Dec 12, 2023

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023 • edited Loading

dime10 commented Dec 12, 2023 • edited Loading

vincentmr commented Dec 12, 2023

erick-xanadu commented Dec 12, 2023

erick-xanadu commented Nov 24, 2023 •

edited by dime10

Loading

codecov bot commented Nov 24, 2023 •

edited

Loading

erick-xanadu commented Dec 12, 2023 •

edited

Loading

erick-xanadu commented Dec 12, 2023 •

edited

Loading

dime10 commented Dec 12, 2023 •

edited

Loading