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

♻️ compute parallelism feature as described in arXiv:2202.11045 #902

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

burgholzer
Copy link
Contributor

As discussed in #895, this PR refactors the implementation of the parallelism feature to align with the description from the original paper https://arxiv.org/abs/2202.11045. The implementation takes extra care to avoid corner cases where divisions by zero could happen.

Fixes #895

Copy link
Member

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

LGTM!

@vtomole
Copy link
Member

vtomole commented Feb 15, 2024

You'll need to add or modify an already existing test to get through the coverage check.

@stephanielee9
Copy link
Contributor

Hi @burgholzer, do you have any updates on this PR?

Co-authored-by: Nils Quetschlich <nils.quetschlich@tum.de>
Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
Copy link
Contributor Author

Hi @burgholzer, do you have any updates on this PR?

Sorry for the long wait here. Somehow the message above got lost in a flood of emails.
Just added the necessary tests and rebased on main.

@vtomole
Copy link
Member

vtomole commented Mar 20, 2024

@burgholzer Looks like 1 more line needs to be covered by the test.

@burgholzer
Copy link
Contributor Author

@burgholzer Looks like 1 more line needs to be covered by the test.

Yeah. That's on me for writing a stupid test 😓 (the cirq.Circuit() one also triggers the num_qubits<=1 condition)
I am having a bit of trouble creating a 0-depth circuit with more than 1 qubit using Cirq. Do you happen to know if there is any way to do that?
In Qiskit, I would just do QuantumCircuit(2) and that would be it and it would cover the remaining missing line.

@vtomole
Copy link
Member

vtomole commented Mar 20, 2024

Create a test in converters_test.py that passes a Qiskit circuit into compute_parallelism_with_qiskit to get that line covered.

Signed-off-by: burgholzer <burgholzer@me.com>
@burgholzer
Copy link
Contributor Author

Create a test in converters_test.py that passes a Qiskit circuit into compute_parallelism_with_qiskit to get that line covered.

Alright. That should do it. Didn't wanna add tests in that file at first as there were basically no related tests in that file.

@vtomole vtomole merged commit 5dfb4b2 into Infleqtion:main Mar 21, 2024
16 of 18 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.

Discussion: Parallelism SupermarQ Metric
3 participants