Skip to content
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

beginning of biased pauli noise for PF sim #295

Merged
merged 16 commits into from
Jul 30, 2024

Conversation

amicciche
Copy link
Member

This is the code that was mentioned in our Zulip messages. One note, it seemed to be that line 41 in the noise.jl, n = nqubits(s) wasn't doing anything, so I went ahead and deleted that in addition to the functions I added.

As mentioned over Zulip, because of the error, this code is untested, aside from being able to generate the object from the Julia REPL.

Copy link

codecov bot commented Jun 19, 2024

Codecov Report

Attention: Patch coverage is 2.77778% with 35 lines in your changes missing coverage. Please review.

Project coverage is 82.24%. Comparing base (1fb1783) to head (e6548f9).

Files Patch % Lines
src/noise.jl 0.00% 18 Missing ⚠️
src/pauli_frames.jl 0.00% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #295      +/-   ##
==========================================
- Coverage   83.01%   82.24%   -0.77%     
==========================================
  Files          61       61              
  Lines        4033     4067      +34     
==========================================
- Hits         3348     3345       -3     
- Misses        685      722      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@amicciche
Copy link
Member Author

I'm now having this error when I try to use QuantumClifford:

image

@Krastanov
Copy link
Member

yeah, tests fail, so I have definitely done something silly. Should have waited for tests to pass before pinging you

@Krastanov
Copy link
Member

it is fixed now

@amicciche
Copy link
Member Author

amicciche commented Jun 21, 2024

The applynoise!() that takes a PauliFrame seems to work correctly, however with the way the constructors are written for PauliError, one can't do something like the following (which will probably a common enough case to warrant fixing):
image
I tried to fix this by changing Integer to Real on line 37 of noise.jl, but this caused an error. Here:
image

@Krastanov
Copy link
Member

What error did the change to Real cause?

@Krastanov Krastanov marked this pull request as draft June 22, 2024 01:04
@Krastanov
Copy link
Member

marking this as a draft just to make my review queue a bit more manageable. Mark it as finished when ready for review.

@amicciche
Copy link
Member Author

What error did the change to Real cause?

When I change it to Real:
image

I get this error:
image

@Krastanov
Copy link
Member

Float is a Real so when you do PauliNoise(x::Real, y::Real, z::Real) = PauliNoise(float(x), float(y), float(z)) you end up with infinite recursion.

A cleaner solution than both what I have written and what you have written is to specify the type for the constructor:

julia> struct A{T} a::T end

julia> A(a::Real) = A{Float64}(a)
A

julia> A(1)
A{Float64}(1.0)

Or even better, something that does not hardcode Float64

julia> struct A{T} a::T end

julia> function A(a::Real) 
           a = float(a)
           return A{typeof(a)}(a)
       end
A

julia> A(1)
A{Float64}(1.0)

julia> A(BigInt(1))
A{BigFloat}(1.0)

So the cleanest solution should be something along the lines of

function PauliNoise(px::Real, py::Real, pz::Real)
    px, py, pz = float.((px, py, pz))
    px, py, pz = promote(px, py, pz)
    T = typeof(px)
    return PauliNoise{T}(px, py, pz)
end

@amicciche
Copy link
Member Author

Ah, I see! I'll try to fix this later today, using what you suggested, thanks!

@amicciche
Copy link
Member Author

amicciche commented Jun 23, 2024

Your code worked as provided! Thanks! With that, I think everything works. I checked for a few codes that doing PauliError(qubit, p) after decoding results in the same logical vs physical error rate plots as PauliError(qubit, p/3, p/3, p/3). For some CSS codes, I also checked that the X and Z logical error corresponding to those simulations also matches doing PauliError(qubit, p*2/3, 0, 0) and PauliError(qubit, 0, 0, p*2/3) correspondingly (and the other X/Z logical error is 0). Similarly, PauliError(qubit, 0, p*2/3, 0) gives the same performance as PauliError(qubit, p).

Copy link
Member

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also update the changelog

src/noise.jl Outdated Show resolved Hide resolved
src/noise.jl Outdated Show resolved Hide resolved
amicciche and others added 4 commits July 2, 2024 09:49
Co-authored-by: Stefan Krastanov <github.acc@krastanov.org>
Co-authored-by: Stefan Krastanov <github.acc@krastanov.org>
@amicciche amicciche marked this pull request as ready for review July 2, 2024 14:40
@amicciche amicciche requested a review from Krastanov July 21, 2024 13:50
@Krastanov Krastanov merged commit f5b8b47 into QuantumSavory:master Jul 30, 2024
12 of 15 checks passed
@amicciche amicciche deleted the biased_pauli_frame_noise branch September 2, 2024 20:42
Fe-r-oz pushed a commit to Fe-r-oz/QuantumClifford.jl that referenced this pull request Sep 9, 2024
Co-authored-by: Stefan Krastanov <github.acc@krastanov.org>
Co-authored-by: Stefan Krastanov <stefan@krastanov.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants