-
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 ignore_measurement_results=True for subcircuits #4760
Conversation
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.
Seeing as the state vector simulator is unaffected by this, there should be no backwards-compatibility issues if users happen to define their own ActOnArgs outside Cirq.
Some docs comments, then this can merge.
Automerge cancelled: A required status check is not present. Missing statuses: ['Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage'] |
Dear, sweet @CirqBot. Your ruthless vigilance towards "incomplete" tests inspires us all 😛 |
Density matrix simulator has a bug that if `ignore_measurement_results` is set, that setting is overlooked within subcircuits. The reason is that this setting check is done in `SimulatorBase.core_iterator`, but that only iterates the outermost circuit. The fix is to move the check into `ActOnArgs.measure`, to ensure that any measurement is replaced with a dephase operation. Surprisingly we did not even have a test for the non-subcircuit case. This PR adds a test for that and the subcircuit case.
Density matrix simulator has a bug that if `ignore_measurement_results` is set, that setting is overlooked within subcircuits. The reason is that this setting check is done in `SimulatorBase.core_iterator`, but that only iterates the outermost circuit. The fix is to move the check into `ActOnArgs.measure`, to ensure that any measurement is replaced with a dephase operation. Surprisingly we did not even have a test for the non-subcircuit case. This PR adds a test for that and the subcircuit case.
Density matrix simulator has a bug that if `ignore_measurement_results` is set, that setting is overlooked within subcircuits. The reason is that this setting check is done in `SimulatorBase.core_iterator`, but that only iterates the outermost circuit. The fix is to move the check into `ActOnArgs.measure`, to ensure that any measurement is replaced with a dephase operation. Surprisingly we did not even have a test for the non-subcircuit case. This PR adds a test for that and the subcircuit case.
Density matrix simulator has a bug that if
ignore_measurement_results
is set, that setting is overlooked within subcircuits.The reason is that this setting check is done in
SimulatorBase.core_iterator
, but that only iterates the outermost circuit.The fix is to move the check into
ActOnArgs.measure
, to ensure that any measurement is replaced with a dephase operation.Surprisingly we did not even have a test for the non-subcircuit case. This PR adds a test for that and the subcircuit case.