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

Move transpiler from Qibolab to Qibo #1055

Merged
merged 70 commits into from
Nov 12, 2023
Merged

Move transpiler from Qibolab to Qibo #1055

merged 70 commits into from
Nov 12, 2023

Conversation

Simone-Bordoni
Copy link
Contributor

@Simone-Bordoni Simone-Bordoni commented Oct 24, 2023

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@Simone-Bordoni Simone-Bordoni self-assigned this Oct 24, 2023
@Simone-Bordoni Simone-Bordoni linked an issue Oct 24, 2023 that may be closed by this pull request
@Simone-Bordoni Simone-Bordoni marked this pull request as ready for review October 30, 2023 09:47
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
src/qibo/transpiler/__init__.py 100.00% <100.00%> (ø)
src/qibo/transpiler/abstract.py 100.00% <100.00%> (ø)
src/qibo/transpiler/blocks.py 100.00% <100.00%> (ø)
src/qibo/transpiler/exceptions.py 100.00% <100.00%> (ø)
src/qibo/transpiler/optimizer.py 100.00% <100.00%> (ø)
src/qibo/transpiler/pipeline.py 100.00% <100.00%> (ø)
src/qibo/transpiler/placer.py 100.00% <100.00%> (ø)
src/qibo/transpiler/router.py 100.00% <100.00%> (ø)
src/qibo/transpiler/star_connectivity.py 100.00% <100.00%> (ø)
src/qibo/transpiler/unitary_decompositions.py 100.00% <100.00%> (ø)
... and 1 more

📢 Thoughts on this report? Let us know!

@Simone-Bordoni Simone-Bordoni linked an issue Nov 1, 2023 that may be closed by this pull request
@renatomello renatomello self-requested a review November 1, 2023 09:18
@Simone-Bordoni
Copy link
Contributor Author

With the new modifications Sabre router can also accept measurements gates

@renatomello renatomello changed the title Move transpiler from Qibolab to `Qibo Move transpiler from Qibolab to Qibo Nov 8, 2023
@stavros11
Copy link
Member

@renatomello @BrunoLiegiBastonLiegi let me know if you agree with merging this. It is blocking the corresponding PR in qibolab and a few other issues there that need to be moved here.

@renatomello
Copy link
Contributor

@renatomello @BrunoLiegiBastonLiegi let me know if you agree with merging this. It is blocking the corresponding PR in qibolab and a few other issues there that need to be moved here.

I improved the docstring and other parts of the code. But there are still a few major things that need to be resolved. Mainly, the transpiler needs to be included in the API reference and also the handling of RNG seeds (at least in the router module). If the merger of this PR is urgent, I could approve it and merge it, and leave those things to be fixed in separate PRs.

src/qibo/transpiler/pipeline.py Outdated Show resolved Hide resolved
src/qibo/transpiler/pipeline.py Show resolved Hide resolved
src/qibo/transpiler/pipeline.py Outdated Show resolved Hide resolved
src/qibo/transpiler/pipeline.py Show resolved Hide resolved
src/qibo/transpiler/placer.py Outdated Show resolved Hide resolved
src/qibo/transpiler/router.py Outdated Show resolved Hide resolved
src/qibo/transpiler/router.py Outdated Show resolved Hide resolved
@stavros11
Copy link
Member

I improved the docstring and other parts of the code. But there are still a few major things that need to be resolved. Mainly, the transpiler needs to be included in the API reference and also the handling of RNG seeds (at least in the router module). If the merger of this PR is urgent, I could approve it and merge it, and leave those things to be fixed in separate PRs.

It is not urgent as in we will use it immediately (the Sabre part doesn't work anyway) but there are several issues in qibolab (qiboteam/qibolab#606, qiboteam/qibolab#643), including by external people, that I would like to move here and start addressing. Also, updates pushed directly to this PR are practically not reviewed because in the file change everything appears as new. Usually it is better to separate moving (copy-pasting) big chunks of code from new developments for this reason.

Regarding the above points, including in the API reference sounds easy to do. For the RNG seeds if it is complicated (as in changing many lines of code) I would rather do in a separate PR or just open an issue and assign to someone. Keep in mind that there are other issues related to this code (some in qiboteam/qibolab#606) that may be more urgent, from a usability point of view.

@renatomello renatomello merged commit 7b744ba into master Nov 12, 2023
21 checks passed
@renatomello renatomello deleted the add_transpiler branch November 12, 2023 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Two qubits unitary decomposition not working for identity Move transpiler to Qibo
5 participants