Skip to content

Conversation

@johnzl-777
Copy link
Contributor

@johnzl-777 johnzl-777 commented Apr 7, 2025

This is an implementation of the address analysis pass for the squin dialect with unit tests added.

A few things to keep in mind:

  • @weinbe58 has helped me identify a problem in kirin with the constant propagation pass handling multiple return values from the Apply statement in squin. He's helped me implement a simple workaround for now as a new method table but there's a more concrete solution here
  • I had move the DAG Analysis into squin but I'm not sure we want to keep that there going forward, I just needed to stop this (rather hard to figure out) circular import from happening when I had to add from bloqade import squin into the impls.py.
  • I remember when @Roger-luo was helping me build this out there was a moment where I was trying to use @wraps on Apply but there was some problem we ran into that required me to hold off on wrapping the squin.wire dialect. I don't quite recall the specifics but I think it would be worth making an issue to keep track of it so I can go ahead and wrap things nicely when it's fixed (:
  • There's also a stopgap Complex type that should be removed when upgrading to the newer version of Kirin which does have the proper type

┆Issue is synchronized with this Jira Task by Unito

@johnzl-777 johnzl-777 requested a review from Roger-luo April 7, 2025 19:38
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

PR Preview Action v1.6.0
Preview removed because the pull request was closed.
2025-04-08 14:08 UTC

@weinbe58
Copy link
Member

weinbe58 commented Apr 7, 2025

For the 'wraps' issue the problem is we do not support unpacking multiple return values when lowering from python to kirin. As such there is no way to support a python frontend for 'wire.Apply'.

@codecov
Copy link

codecov bot commented Apr 7, 2025

Codecov Report

Attention: Patch coverage is 85.10638% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/bloqade/analysis/address/impls.py 80.95% 4 Missing ⚠️
src/bloqade/analysis/address/lattice.py 62.50% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
5523 4743 86% 0% 🟢

New Files

File Coverage Status
src/bloqade/squin/analysis/_init_.py 100% 🟢
src/bloqade/squin/op/complex.py 100% 🟢
TOTAL 100% 🟢

Modified Files

File Coverage Status
src/bloqade/analysis/address/_init_.py 100% 🟢
src/bloqade/analysis/address/analysis.py 93% 🟢
src/bloqade/analysis/address/impls.py 63% 🟢
src/bloqade/analysis/address/lattice.py 85% 🟢
src/bloqade/qasm2/dialects/glob.py 100% 🟢
src/bloqade/qasm2/dialects/parallel.py 86% 🟢
src/bloqade/qasm2/dialects/uop/schedule.py 93% 🟢
src/bloqade/qasm2/passes/parallel.py 98% 🟢
src/bloqade/qasm2/rewrite/uop_to_parallel.py 69% 🟢
src/bloqade/squin/op/stmts.py 100% 🟢
src/bloqade/squin/wire.py 100% 🟢
TOTAL 90% 🟢

updated for commit: a637d95 by action🐍

@johnzl-777
Copy link
Contributor Author

@weinbe58 got it, I'll remove the attempt at wrapping I commented out. I assume this shouldn't be a problem anyways because it isn't our intent to let people construct entire circuits with the squin dialect

@weinbe58
Copy link
Member

weinbe58 commented Apr 7, 2025

Actually we don't have a rewrite from qubits to wire dialect @johnzl-777 so writing anything is going to be difficult.

@Roger-luo we either need to implement python lowering multiple return values or implement the rewrite pass from the qubits to the wire dialect. ideally we would have both since you can get slightly more optimal code using the wires especially with subroutine calls

@Roger-luo Roger-luo merged commit 58b65c3 into main Apr 8, 2025
11 of 12 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.

4 participants