noncliff: add copy, == for GeneralizedStabilizer, UnitaryPauliChannel #419
Conversation
src/nonclifford.jl
Outdated
| @@ -342,6 +347,8 @@ struct UnitaryPauliChannel{T,S,P} <: AbstractPauliChannel | |||
| end | |||
|
|
|||
| PauliChannel(p::UnitaryPauliChannel) = p.paulichannel | |||
| Base.copy(p::UnitaryPauliChannel) = deepcopy(p) | |||
There was a problem hiding this comment.
using deepcopy as an implementation of copy is a bit dangerous. It frequently works fine, but if you have views into some other memory it can produce unexpected results. The more boilerplate-heavy "Constructor on copied slot entries" is much safer here.
There was a problem hiding this comment.
Since tuples are immutable, I used p.weights or p.paulis instead of copy(p.weights), copy(p.paulis) etc.
| @@ -342,6 +347,8 @@ struct UnitaryPauliChannel{T,S,P} <: AbstractPauliChannel | |||
| end | |||
|
|
|||
| PauliChannel(p::UnitaryPauliChannel) = p.paulichannel | |||
| Base.copy(p::UnitaryPauliChannel) = UnitaryPauliChannel(p.paulis, p.weights) | |||
| Base.:(==)(p₁::UnitaryPauliChannel, p₂::UnitaryPauliChannel) = p₁.paulis==p₂.paulis && p₁.weights==p₂.weights | |||
There was a problem hiding this comment.
The tuples might be immutable, but the objects they contain are not-necessarily immutable. E.g. if I do something stupid like mul_left!(p.paulis, some_other_pauli) on a copied p, the original p would also be modified. I am stupid to do that to begin with, but nonetheless I would be right to be surprised that modifying the copy modified the original as well.
More generally, let the compiler worry about immutability unless you know for a fact that the compiler is doing a bad job. The compiler can figure out when immutability guarantees let it reuse memory:
Lastly, you are probably doing this because Tuple does not have a copy method. That is a frustration I have with julia -- the story behind it can be found starting in this discussion: https://discourse.julialang.org/t/why-can-i-deepcopy-nothing-but-not-copy-nothing/112429 (in particular see the links by stevengj).
Anyway, to make sure this is an actual copy, the clunky but necessary map(copy, p.paulis) can be used.
There was a problem hiding this comment.
Thank you for these insights! I avoided using copy(p.paulis) so it wouldn’t look stupid.
|
thank you! |
This PR is inspired from this comment: #355 (comment)