-
Notifications
You must be signed in to change notification settings - Fork 11
Naive crosstalk #175
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
Naive crosstalk #175
Conversation
| // outcomes to the OutputEngine, which should be ignored. How do we do this? | ||
| // Perhaps there should be another add_collapse() method that applies a | ||
| // measurement and discards the outcome. | ||
| builder.add_measurements(&affected_qubits); |
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.
Comment from Ciaran on the commit:
Hmmmm, let's see. Measurements end up kinda of being two rounds... First the noise model sends a measurement request to the simulator and then noise model gets a result from the simulator. Then then noise model can intercept/forward/etc. results to the "classical engine". This is done here:
PECOS/crates/pecos-engines/src/noise/general.rs
Line 362 in 55994b8
fn continue_processing(I kinda of had to deal with a similar situation of distinguishing measurement results for leakage... and I ended up just storing the measurement type here:
PECOS/crates/pecos-engines/src/noise/general.rs
Line 332 in 55994b8
measured_qubits: Vec<(usize, bool)>,But I feel like what needs to be done is to add some tagging mechanism in ByteMessage
I might... try adding result tagging to ByteMessage...
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.
Ok, I think I understand. Without tagging in ByteMessage, what I could do is add an extra bool to measured_qubits, so that's Vec<(usize, bool, bool)> where the second bool corresponds to is_crosstalk to indicate its origin.
For the naive crosstalk, I'd just throw away the measurement outcome at apply_noise_on_continue_processing here whenever is_crosstalk is True. In future versions, it could be used to make a decision on how to apply crosstalk.
That makes sense, I can do it tomorrow morning. I guess there's no need for tags for the first version, but if you get them in, I'll use them.
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.
Yeah, sounds good. Later I can add a tag system and replace that.
| p2_idle: 0.0, | ||
| leaked_qubits: HashSet::new(), | ||
| rng: NoiseRng::default(), | ||
| initialized_qubits: BTreeSet::new(), |
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.
Comment from Ciaran in the commit:
dcc5fb7#commitcomment-164014201
Would prefer initialized -> prep or prepared or something
Note: crosstalk for measurement and prep/reset/init can look differently. But can change that later.
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.
Sure, happy to change that.
ciaranra
left a comment
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
Creating a PR just so that it's easier to track conversations.