-
Notifications
You must be signed in to change notification settings - Fork 17
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
Added Testing for 3to1 purification #29
Conversation
Codecov Report
@@ Coverage Diff @@
## master #29 +/- ##
==========================================
- Coverage 67.80% 67.63% -0.17%
==========================================
Files 23 23
Lines 702 893 +191
==========================================
+ Hits 476 604 +128
- Misses 226 289 +63
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
test/test_circuitzoo_purification.jl
Outdated
for i in 1:13 | ||
initialize!(r[(2*i-1):(2*i)], bell) | ||
end | ||
@test PurifyStringent()(r[1], r[2], |
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.
Probably a bit simpler to do with (r[1:26]...)
instead of giving each slot separately.
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.
When i do this the whole library gets kill signal because of the lack of ram.
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.
Oh, interesting, that is certainly a bug that should be fixed with the base library. Could you file an issue for it?
There is another quick fix: use the Clifford representation instead of state vectors. Kets and density matrices are exponentially expensive (as you have observed), but a Clifford tableaux simulation should be much cheaper and faster (even if it can not simulate all the types of dynamics that a ket-style simulation can)
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.
But there seems to be a problem when measuring the fidelity. It only works in QuantumOpticsRepr else it yields an error.
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.
Expression: abs(observable(r[1:2], projector(bell))) >= rnd
DomainError with Only pure (not mixed) states are supported when calculating inner product.:
test/test_circuitzoo_purification.jl
Outdated
apply!(r[target], Dict(:X=>X, :Y=>Y, :Z=>Z)[error]) | ||
@test Purify3to1(fixtwice)(r[1], r[2], [r[3], r[5]], [r[4], r[6]])==false | ||
end | ||
## When [error, fixtwice] in {[X,Z], [Z,Y], [Y,X]} it yields true is that supposed to happen? |
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.
Could you elaborate, what does that mean?
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 apparently it does not detect the error when [error, fixtwice] are in {[X,Z], [Z,Y], [Y,X]}. It seems a quite consistent behaviour and i think it's supposed to happen but i dont seem to figure out why
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.
This is in the 3to1 circuit, right?
We looked at a couple of versions of the 3to1 circuit. Some that can easily be split into two 2to1 circuits, where pair 3 is sacrificed to purify pair 2, which is sacrificed to purify pair 1. But there are also 3to1 circuits which can not be split neatly like into separate subcircuit -- all pair have gates between them before measurements can start. You observed that both of these styles of circuits have the same performance if the gates are perfect. That had confused me at first -- because the non-factorable circuits are known to be able to fight also gate errors.
Could you try to run that simulation? One in which gate errors also happen. That would let us know whether we are missing some better circuits.
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 ran it and the double selection circuits do have better fidelity then the ordered ones.
The notebook is in a repo in my profile
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.
Can you provide some feedback on testing? |
@Barosandu , left in two comments that I hope are helpful. One is for how to make it a bit faster to run some of the simulations (by sticking to only quantumclifford.jl representation), one about reporting as a bug some of the issues you uncovered, and one about verifying the 3to1 circuits are the ones we want. |
@Krastanov i committed the final touches. Please review :) |
src/CircuitZoo/CircuitZoo.jl
Outdated
@@ -132,4 +132,431 @@ function (circuit::Purify2to1)(purifiedL,purifiedR,sacrificedL,sacrificedR) | |||
success | |||
end | |||
|
|||
struct Purify2to1Left <: AbstractCircuit |
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.
could you:
- add a detailed docstring for this circuit, including tests
- rename it to Purify2to1Node (otherwise folks will be wondering where is the
Right
one)
leaveout::Symbol | ||
function Purify2to1Left(leaveout) | ||
if leaveout ∉ (:X, :Y, :Z) | ||
throw(ArgumentError(lazy""" |
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.
could you add a @test_throws
in the tests so this line is also tested?
src/CircuitZoo/CircuitZoo.jl
Outdated
``` | ||
|
||
""" | ||
|
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.
remove empty lines between docstrings and types
src/CircuitZoo/CircuitZoo.jl
Outdated
(success1 && success2) | ||
end | ||
|
||
struct Purify3to1Left <: AbstractCircuit |
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.
docstring
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.
and rename similarly to previous
measa | ||
end | ||
|
||
struct StringentHead <: AbstractCircuit |
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.
docstring
src/CircuitZoo/CircuitZoo.jl
Outdated
success | ||
end | ||
|
||
struct StringentHeadLeft <: AbstractCircuit |
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.
docstring and rename
[coinleft(σˣ, [sacrificedL[1]]), coinleft(σˣ, [sacrificedL[2]])] | ||
end | ||
|
||
struct StringentBody <: AbstractCircuit |
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.
docstring
src/CircuitZoo/CircuitZoo.jl
Outdated
|
||
end | ||
|
||
struct StringentBodyLeft <: AbstractCircuit |
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.
docstring and rename
src/CircuitZoo/CircuitZoo.jl
Outdated
``` | ||
|
||
""" | ||
|
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.
remove empty line between docstring and type definition
success | ||
end | ||
|
||
struct PurifyExpedient <: AbstractCircuit |
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.
docstring
src/CircuitZoo/CircuitZoo.jl
Outdated
end | ||
|
||
|
||
struct PurifyStringentLeft <: AbstractCircuit |
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.
docstring and rename
src/CircuitZoo/CircuitZoo.jl
Outdated
[a..., b..., c..., d...] | ||
end | ||
|
||
struct PurifyExpedientLeft <: AbstractCircuit |
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.
docstring and rename
This looks pretty great and it is very close to merging. Could you make the changes I suggested and we can discuss the tests you have implemented at the next in-person meeting? |
I ve done all the renamings and docstrings @Krastanov , and also added EXPEDIENT testing |
for i in 1:13 | ||
initialize!(r[(2*i-1):(2*i)], bell) | ||
end | ||
@test PurifyStringent()(r[1], r[2], r[3:2:25], r[4:2:26]) == 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.
test the observable equals to one
end | ||
|
||
@testset "Stringent - Fidelity - CliffordRepr" begin | ||
for rep in [CliffordRepr] |
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.
delete
test/test_circuitzoo_purification.jl
Outdated
initialize!(r[(2*i-1):(2*i)], noisy_pair) | ||
end | ||
if PurifyStringent()(r[1], r[2], r[3:2:25], r[4:2:26]) == true | ||
@test_broken observable(r[1:2], projector(bell)) ≈ 0.0 |
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.
This is a probabilistic test. It has a small chance of triggering
end | ||
|
||
@testset "Expedient - Fidelity - CliffordRepr" begin | ||
for rep in [CliffordRepr] |
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.
same as expedient
test/test_circuitzoo_purification.jl
Outdated
@@ -35,3 +55,312 @@ const bell = StabilizerState("XX ZZ") | |||
end | |||
end | |||
end | |||
|
|||
@testset "2to1 - Single Qubit" begin |
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.
rename 2to1 Node
test/test_circuitzoo_purification.jl
Outdated
initialize!(r[1:6], bell⊗bell⊗bell) | ||
ma = Purify3to1Node(fixtwice)(r[1], [r[3],r[5]]) | ||
mb = Purify3to1Node(fixtwice)(r[2], [r[4],r[6]]) | ||
@test ma == mb |
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.
delete
apply!(r[target], Dict(:X=>X, :Y=>Y, :Z=>Z)[error]) | ||
|
||
if Dict(:X=>:Z, :Y=>:X, :Z=>:Y)[error] == fixtwice | ||
@test Purify3to1(fixtwice)(r[1], r[2], [r[3], r[5]], [r[4], r[6]])==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.
create an issue on github about parametrization on 3to1 circuits
No description provided.