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.

Sign up for GitHub

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

Jump to bottom

Parallel ops #53

Merged
merged 30 commits into from
Dec 12, 2023
Merged

Parallel ops #53

merged 30 commits into from
Dec 12, 2023

Conversation

Asa-Kosto-QTM
Copy link
Collaborator

Parallel ops in PHIR
parallel ops are default option when there is a machine object

@qartik
Copy link
Member

qartik commented Dec 6, 2023

I am going to make a new release of phir so that the CI can go through.

qartik
qartik requested changes Dec 6, 2023
View reviewed changes
pytket/phir/cli.py Outdated Show resolved Hide resolved
pytket/phir/phirgen.py Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Show resolved Hide resolved
@qartik
Copy link
Member

qartik commented Dec 6, 2023

I also recommend adding a few specific test cases for testing the new parallel functionality.

qartik
qartik requested changes Dec 6, 2023
View reviewed changes
Copy link
Member

@qartik qartik left a comment

Choose a reason for hiding this comment

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

Another set of lint-related suggestions. Will do another round of review later.

pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
@Asa-Kosto-QTM @qartik
remove unused value
39c499a 
Co-authored-by: Kartik Singhal <kartik.singhal@quantinuum.com>
qartik
qartik requested changes Dec 6, 2023
View reviewed changes
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
pytket/phir/phirgen_parallel.py Outdated Show resolved Hide resolved
@qciaran qciaran self-requested a review December 6, 2023 21:06
qartik
qartik reviewed Dec 6, 2023
View reviewed changes
pytket/phir/phirgen.py Outdated Show resolved Hide resolved
qartik
qartik previously approved these changes Dec 8, 2023
View reviewed changes
qartik
qartik previously approved these changes Dec 11, 2023
View reviewed changes
dlucchetti
dlucchetti requested changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@dlucchetti dlucchetti left a comment

Choose a reason for hiding this comment

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

Support for TK2 similar to other two qubit gates is needed for full parallelization unless we want to delay for an independent delivery.

pytket/phir/phirgen_parallel.py Show resolved Hide resolved
dlucchetti
dlucchetti previously approved these changes Dec 12, 2023
View reviewed changes
@Asa-Kosto-QTM Asa-Kosto-QTM dismissed stale reviews from dlucchetti and qartik via 5b6534f December 12, 2023 18:05
qartik
qartik requested changes Dec 12, 2023
View reviewed changes
tests/test_pll_tk2.py Outdated Show resolved Hide resolved
tests/data/qasm/pll_test.qasm Outdated Show resolved Hide resolved
@qartik qartik merged commit e8c40de into main Dec 12, 2023
2 checks passed
@qartik qartik deleted the parallel-ops branch December 12, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qartik qartik qartik approved these changes

@dlucchetti dlucchetti dlucchetti left review comments

@peter-campora peter-campora Awaiting requested review from peter-campora

@qciaran qciaran Awaiting requested review from qciaran

@neal-erickson neal-erickson Awaiting requested review from neal-erickson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Asa-Kosto-QTM @qartik @dlucchetti @qciaran