-
Notifications
You must be signed in to change notification settings - Fork 65
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
Defining dephasing and depolarizing operators with projectors #715
Conversation
@@ -122,7 +133,7 @@ def basis_check(noise_type: str) -> None: | |||
if self.basis_name == "digital" | |||
else config.dephasing_rate | |||
) | |||
local_collapse_ops.append(np.sqrt(rate / 2) * qutip.sigmaz()) | |||
local_collapse_ops.append(np.sqrt(rate / 2) * pauli_2d["z"]) |
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 agree with the current implementation. For scalability, I think we once discussed it was possible to define the collapse operator for dephasing channel as:
local_collapse_ops.append(np.sqrt(rate / 2) * pauli_2d["z"]) | |
state, rate = "h", config.hyperfine_dephasing_rate if self.basis_name =="digital" else "r", config.dephasing_rate | |
local_collapse_ops.append(np.sqrt(rate) * self.op_matrix[f"sigma_{a}{a}"]) |
But that will imply changing the tests...
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 also keep this idea for a future PR where we implement noise in 'all' channel (which I guess we should implement before tackling the implementation of "leakage").
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 agree with the current implementation. For scalability, I think we once discussed it was possible to define the collapse operator for dephasing channel as:
But that will imply changing the tests...
I'm pretty sure this is true for Kraus operators but I don't remember checking it also applies for collapse operators.
We can also keep this idea for a future PR where we implement noise in 'all' channel (which I guess we should implement before tackling the implementation of "leakage")
Yeah, we still have to figure out how we'll do that...
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.
Well if it works for kraus it can also be extended to collapse operators :)
I think that was the core of what I had in mind when I created this issue (I had in mind to first implement effective noise operators with 'all' and then move on and dephasing and depolarizing with "all"). I propose to tackle the simulation with effective noise operators in "all" basis in the next PR.
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.
Perhaps we should resume this PR after having implemented the simulation with effective noise operators in 'all' basis (and having in mind the simulations with leakage loise), since at the moment the feature is nice but it does not add much value (as you commented in the issue). What do you think ?
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.
Well if it works for kraus it can also be extended to collapse operators :)
I know there's always a way to go from one to the other, but is it obvious that the operators are always the same and it's just the probability that changes into a rate?
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.
Perhaps we should resume this PR after having implemented the simulation with effective noise operators in 'all' basis (and having in mind the simulations with leakage loise), since at the moment the feature is nice but it does not add much value (as you commented in the issue). What do you think ?
Well, this is already done so I guess we might as well merge it and then make the necessary modifications once "all" is supported
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 agree, I had not seen your comments in the other PR :)
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
Fixes #691 .