-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix str/repr explosion in separated states #4518
Conversation
@95-martin-orion FWIW I think before cirq 1.0 we should get rid of all the extra MySimulatorStepResult and MySimulatorTrialResult classes, and just yield/return TActOnArgs as the result (renamed to TSimulatorState). I don't see any value added by the various StepResult and TrialResult classes, but they add a bunch of unnecessary code obfuscation. Obviously a big breaking change..... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to go. Let me know when you're ready to merge, in case there's any issues colliding with your other PRs in flight.
Could you open an issue and CC @MichaelBroughton on it? There's a laundry list of TODOs like this somewhere that he's been keeping track of. |
This can be merged.
…On Mon, Dec 20, 2021 at 2:20 PM Orion Martin ***@***.***> wrote:
***@***.**** approved this pull request.
Looks good to go. Let me know when you're ready to merge, in case there's
any issues colliding with your other PRs in flight.
—
Reply to this email directly, view it on GitHub
<#4518 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG464OAPY4CRFR32XD7EA3UR6T4JANCNFSM5ET4AEAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Automerge cancelled: A status check is failing. |
Looks like the |
Looks like an ongoing Docker outage is breaking us: https://downdetector.com/status/docker/ We'll need to wait it out before merging. |
Looks like it's up again |
* Fix str/repr explosion in separated states * format * Fix str/repr explosion in state vector * format * nondirac_str * TrialResultBase * cleanup * Add get_state_containing_qubit * Ensure eval(repr(obj)) holds * substates not a property * fix test * coverage * test * fix windows Co-authored-by: Cirq Bot <craiggidney+github+cirqbot@google.com>
Hi @daxfohl , I tried running the following:
Is the phase output expected? I was surprised to see an output vector after "phase" but I might be misunderstanding something. Thanks! |
There is a phase component. Try adding a global phase and see how it changes? I can't remember. Maybe there's a better way to output the phase. |
I appended a
Would it make sense to just have the phase in the output and remove the output vector part? |
Yes that would probably make more sense :) |
* Fix str/repr explosion in separated states * format * Fix str/repr explosion in state vector * format * nondirac_str * TrialResultBase * cleanup * Add get_state_containing_qubit * Ensure eval(repr(obj)) holds * substates not a property * fix test * coverage * test * fix windows Co-authored-by: Cirq Bot <craiggidney+github+cirqbot@google.com>
* Fix str/repr explosion in separated states * format * Fix str/repr explosion in state vector * format * nondirac_str * TrialResultBase * cleanup * Add get_state_containing_qubit * Ensure eval(repr(obj)) holds * substates not a property * fix test * coverage * test * fix windows Co-authored-by: Cirq Bot <craiggidney+github+cirqbot@google.com>
Fixes #4477 by outputting separated states individually rather than kronning them all together first.