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

Dont batch unless separate samplers are used for each subcircuit #333

Merged
merged 23 commits into from
Aug 14, 2023

Conversation

caleb-johnson
Copy link
Collaborator

Resolves #330

@caleb-johnson caleb-johnson added quantum performance Related to the efficiency with which we are able to gain information from the quantum hardware cutting QPD-based circuit cutting code labels Jul 20, 2023
@github-actions
Copy link

github-actions bot commented Jul 20, 2023

Pull Request Test Coverage Report for Build 5623635187

  • 20 of 20 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 90.739%

Totals Coverage Status
Change from base Build 5605572776: 0.03%
Covered Lines: 2567
Relevant Lines: 2829

💛 - Coveralls

num_qpd_bits = np.reshape(
num_qpd_bits_flat, (len(subexperiments), len(subexperiments[0]))
)
quasi_dists_reshaped: list[list[QuasiDistribution]] = [[] for _ in subexperiments]
Copy link
Collaborator Author

@caleb-johnson caleb-johnson Jul 20, 2023

Choose a reason for hiding this comment

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

Can't rely on numpy here anymore because there are now potentially different subcircuits coming in on the same batch, and those can have different numbers of subexperiments associated with them; therefore, we can't trivially reshape to the input size anymore.

We now have to "stream" the quasi-dists into the output data structure, using the input as a guide for how to re-build the structure

@caleb-johnson caleb-johnson requested review from garrison and removed request for garrison July 20, 2023 18:33
@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Jul 21, 2023

If we get this one merged soon, it's probably worth a patch release to 0.2.1 due to the large amount of time saved in cases where the circuit is separated and the user runs on quantum hardware with no reservation

@garrison
Copy link
Member

If we get this one merged soon, it's probably worth a patch release to 0.2.1 due to the large amount of time saved in cases where the circuit is separated and the user runs on quantum hardware with no reservation

That reminds me: this definitely deserves a release note.

@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Jul 21, 2023

If we get this one merged soon, it's probably worth a patch release to 0.2.1 due to the large amount of time saved in cases where the circuit is separated and the user runs on quantum hardware with no reservation

This thought occurred to me before my coffee. We already have new features in main, it's too late to consider a bugfix release probably :)

garrison and others added 7 commits July 28, 2023 09:29
Now that CKT supports Python 3.11, we might as well use the latest version in the Dockerfile.
* Add support for `SwapGate`

* Reorder terms

[ci skip]

* Add missing terms

* DRY the coefficients

* Fix coverage

* Add support for `iSwapGate`

* Fix black

* Add to release note

* Fix type hint

* Gates without parameters are nicer to work with

and can be singletons, one day!

* Remove a line

* Add comments describing channels

* `_copy_unique_sublists`

* Add `DCXGate`

* Tweak

* Implement cutting of general 2-qubit unitaries

Builds on #294.  Closes #186.

* Add tests of additional gates

* Fix type annotation

* Add explanatory comments

* `supported_gates()` -> `explicitly_supported_gates()`

* Add to references

* Improved error message and test about `to_matrix` conversion failing

* Add xref to `QPDBasis` in docstrings

* Add `qpdbasis_from_gate` to Sphinx build

* Make `explicitly_supported_gates` private and remove its release note

It's not clear that this function remains useful now that we
support essentially all 2-qubit gates.  If we find a use for it
in the future, we can re-introduce it (or something like it) as a
public interface.

* Fix intersphinx link

* Release note

* Update qpd.py: remove extraneous `from None`
I've also tried to make the developer documentation easier to discover
* Make the repository link more obvious from the Sphinx build

* Enable "edit" link in the header

* Add comment
* Add docs badge to link to stable docs

* Add ruff badge

* Remove ruff
Making directions on opening the docs a little more succinct.
caleb-johnson and others added 4 commits July 28, 2023 11:03
* Implement wire cutting as a two-qubit instruction

* Update type annotation

* s/gate/instruction/

* Add overhead test for `Move` instruction

* Add wire cutting tutorial

* Add `Move` to Sphinx build

* Doc updates suggested by Caleb

* Add release note and link to new tutorial

* Clarify wording

following #174 (comment)

* Improvements to `Move` docstring

* Use svg as the plot format

This avoids pixelation on high-dpi displays

* Remove unnecessary uses of `CircuitInstruction`

https://github.com/Qiskit-Extensions/circuit-knitting-toolbox/pull/174/files#r1278067109

* The notebook tests should ignore any files that crop up in `docs/_build`

`matplotlib.sphinxext.plot_directive` likes to leave python files there
@caleb-johnson
Copy link
Collaborator Author

@garrison the change log should now be correct in this PR now, so it is actually reviewable :)

@caleb-johnson caleb-johnson requested a review from garrison August 14, 2023 17:34
Copy link
Member

@garrison garrison left a comment

Choose a reason for hiding this comment

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

Thanks, this looks reasonable, and it appears basically correct as far as I can tell without working out every last detail explicitly about the orderings in these reshaped lists. In order to raise my confidence to a pretty high bar, I tested this with a modified roundtrip test, and everything works out. I will submit my modified test in a PR soon, as I think it is worth doing on an ongoing basis. Thanks!

garrison added a commit that referenced this pull request Aug 14, 2023
This is my PR promised at
#333 (review).

Essentially, I really want the roundtrip tests to regularly test
both experiment workflows: the one that uses a single sampler, and
the one that uses multiple samplers.  Of course, all sampler(s)
used should be `ExactSampler`s, and the expectation values are
required to work out exactly, up to floating point error.

This PR achieves this by flipping a coin each time a roundtrip
test is called.
@caleb-johnson caleb-johnson merged commit 4e0cf63 into main Aug 14, 2023
@caleb-johnson caleb-johnson deleted the dont-batch branch August 14, 2023 22:08
garrison added a commit that referenced this pull request Aug 15, 2023
This is my PR promised at
#333 (review).

Essentially, I really want the roundtrip tests to regularly test
both experiment workflows: the one that uses a single sampler, and
the one that uses multiple samplers.  Of course, all sampler(s)
used should be `ExactSampler`s, and the expectation values are
required to work out exactly, up to floating point error.

This PR achieves this by flipping a coin each time a roundtrip
test is called.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cutting QPD-based circuit cutting code quantum performance Related to the efficiency with which we are able to gain information from the quantum hardware
Projects
None yet
Development

Successfully merging this pull request may close these issues.

execute_experiments should not batch subcircuit experiments unless multiple samplers are passed
3 participants