quantumlib / qsim

Schrödinger and Schrödinger-Feynman simulators for quantum circuits.
Apache License 2.0
454 stars 151 forks source link

Adding Qsim confusion matrix runtime exception #630

Closed angelo-laskaris closed 1 year ago

angelo-laskaris commented 1 year ago

QSim currently does not support confusion matrices on measurement gates. See https://github.com/quantumlib/Cirq/issues/6305 for more info. While work is done to support confusion matrices surfacing a runtime exception.

angelo-laskaris commented 1 year ago

Adding black formatter to my IDE, rebasing and then posting a commit on top of this one

angelo-laskaris commented 1 year ago

@95-martin-orion The one test that failed doesn't seem to be related to the change. Is this test ever flaky?

95-martin-orion commented 1 year ago

Hmmm. This seems related to this SO answer, as the logs indicate that python 3.12.0 was installed for the test. I'm not quite sure how this happened, though, seeing as both setup.py and the Github Actions workflow specify a maximum python version of 3.11.

It's possible we need to pin our python version in the test workflow.

95-martin-orion commented 1 year ago

Opened #632 to track the issue.

95-martin-orion commented 1 year ago

632 is fixed - I'm going to go ahead with the v0.17.0 release to unblock some things, but I'm happy to cut a v0.17.1 once this is merged.

angelo-laskaris commented 1 year ago

632 is fixed - I'm going to go ahead with the v0.17.0 release to unblock some things, but I'm happy to cut a v0.17.1 once this is merged.

Okay sounds good!

tanujkhattar commented 1 year ago

@95-martin-orion Are there any further concerns or can we LGTM?

95-martin-orion commented 1 year ago

@95-martin-orion Are there any further concerns or can we LGTM?

Sorry about the delay - this dropped off my radar. I've unresolved one comment above that I'd like clarity on, otherwise this is good to go. I'll follow up with a release shortly after.

angelo-laskaris commented 1 year ago

@95-martin-orion Are there any further concerns or can we LGTM?

Sorry about the delay - this dropped off my radar. I've unresolved one comment above that I'd like clarity on, otherwise this is good to go. I'll follow up with a release shortly after.

No worries! Followed up on your comment