-
Notifications
You must be signed in to change notification settings - Fork 368
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 snapshot density matrix #374
Conversation
* change density matrix from singleshot to average snapshot * if circuit contains density matrix snapshot it must default to density matrix method * adds State name to error message for invalid circuit instructions
* Add missing switch statement for manually selecting "statevector" qasm method
e28c62a
to
09d5e29
Compare
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.
LGTM!
Statevector::State<QV::QubitVector<float>> state; | ||
validate_state(state, circ, noise_model, true); | ||
} else { | ||
Statevector::State<QV::QubitVector<>> state; |
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.
I think it's clearer if we explicitily say:
Statevector::State<QV::QubitVector<>> state; | |
Statevector::State<QV::QubitVector<double>> state; |
if (validate) { | ||
if (simulation_precision_ == Precision::single_precision) { | ||
Statevector::State<QV::QubitVector<float>> state; | ||
validate_state(state, circ, noise_model, true); |
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.
We can move this validate_state()
right after the en of the conditional, because both branches of the conditional call the same function with the same args.
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.
the state arg is actually different (template param <float>
or <double>
);
return Method::statevector; | ||
} | ||
// If circuit contains invalid instructions for statevector throw a hail | ||
// mary and try for density matrix. |
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.
Ave María!!
@@ -30,6 +30,10 @@ | |||
snapshot_state_pre_measure_statevector_nondeterministic, | |||
snapshot_state_post_measure_statevector_nondeterministic) | |||
|
|||
def get_snapshots(data, label, snapshot_type): | |||
"""Format snapshots as list of Numpy arrays""" |
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.
I think this has one more level of indentation, right?
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.
indeed!
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.
Will merge here and fix in #380
Summary
Fixes density matrix snapshot and adds tests.
Closes #303
Blocked by #355Details and comments