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

Remove Tket dependencies for PennyLane model #195

Merged
merged 53 commits into from
Feb 27, 2025

Conversation

blakewilsonquantinuum
Copy link
Contributor

Currently, to make a pennylane circuit, we construct a tket circuit, extract the ops, and then construct the pennylane circuit from that. This requires tket being a middle man for any circuit conversion. Instead, we can convert to a preliminary diagram compatible with all circuits before converting to the particular backend.

make_circuital function - Built. Needs more testing.
Pulls qubits to the top of the diagram.
Pulls measurements to the bottom of the diagram.
Orders qubits and measurements so they are easy to build a circuit.
Type checks each layer to ensure a valid diagram.
Type checking has been tested on all diagrams from the mc_dataset.
is_circuital - Needs built.
Checks if a diagram is a valid circuital diagram.
Verifies qubits, measurements, and gates are all placed correctly and type checks.
started to build in the draft PR.
from_circuit - Needs built.
Function to build a tket, pennylane, or other circuit using the preliminary circuit.
Refactor tket, pennylane, and torch_quantum to use the above functions for building their circuits.

@dimkart dimkart changed the title Tketdepremoval Remove Tket dependencies for PennyLane model Dec 2, 2024
@blakewilsonquantinuum
Copy link
Contributor Author

The mypy issue above is unrelated to this PR. It comes from grammar.py when returning NotImplemented and is only flagged in the newest version of mypy (1.14.1 released on Dec 30 2024)

image

@dimkart dimkart linked an issue Feb 20, 2025 that may be closed by this pull request
@dimkart
Copy link
Contributor

dimkart commented Feb 21, 2025

@neiljdo Another thing we'd like is to make tket an optional dependency, only related to TketModel.

@neiljdo
Copy link
Collaborator

neiljdo commented Feb 25, 2025

@dimkart I've reduced the wall time for model creation (see below):

Before pickle-hack:
image

After pickle-hack:
image

There's still some discrepancy between the training times - I'm investigating that next.

@dimkart
Copy link
Contributor

dimkart commented Feb 25, 2025

@neiljdo This looks good -- when you finish with the fixes let's attempt another comparison with the previous implementation (sympy symbols + tket in the middle)

@neiljdo
Copy link
Collaborator

neiljdo commented Feb 26, 2025

@dimkart here are the new benchmarking results with the updated PennyLaneModel:

image

pennylanemodel-w-native-symbol.pdf

The TketModel leaves much to be desired regarding training speed - I can investigate this on a later PR.

@dimkart
Copy link
Contributor

dimkart commented Feb 26, 2025

@neiljdo Thanks for the report. The training speed of tket model looks a lot worse compared to previous run, is that normal? Also, how is the performance of the model? Do we get the expected results with the new implementation?

@neiljdo
Copy link
Collaborator

neiljdo commented Feb 27, 2025

@dimkart I've reran the full benchmark and now the TketModel is more comparable to the previous results - I think there might have been some other process that ran during that part of the benchmark.

image

As for the accuracy metrics, it's pretty much the same for all - it's just the wall times that changed.
image

@dimkart
Copy link
Contributor

dimkart commented Feb 27, 2025

@neiljdo Good work, as long as we move tket to the optional dependencies I think this can be merged.

Copy link
Contributor

@dimkart dimkart left a comment

Choose a reason for hiding this comment

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

Looks good now!

@neiljdo neiljdo merged commit fb9dacd into CQCL:main Feb 27, 2025
9 checks passed
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.

Eliminate unnecessary Tket dependencies
3 participants