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

Reducing CNOT count during circuit extraction #70

Merged
merged 21 commits into from
May 14, 2021
Merged

Conversation

ChenZhao44
Copy link
Member

I use SWAP gates to reduce CNOT counts.
More tests are needed. Please do not merge until tests are added.

@codecov
Copy link

codecov bot commented Mar 7, 2021

Codecov Report

Merging #70 (f957110) into master (e9a872a) will increase coverage by 1.41%.
The diff coverage is 87.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   82.18%   83.59%   +1.41%     
==========================================
  Files          11       11              
  Lines        1532     1652     +120     
==========================================
+ Hits         1259     1381     +122     
+ Misses        273      271       -2     
Impacted Files Coverage Δ
src/ir.jl 62.50% <64.70%> (+25.51%) ⬆️
src/simplify.jl 87.59% <92.94%> (+9.33%) ⬆️
src/circuit_extraction.jl 79.56% <100.00%> (+3.22%) ⬆️
src/phase.jl 64.06% <100.00%> (ø)
src/phase_teleportation.jl 100.00% <100.00%> (ø)
src/zx_diagram.jl 84.94% <100.00%> (-9.62%) ⬇️
src/zx_graph.jl 92.14% <100.00%> (ø)
src/rules.jl 89.91% <0.00%> (+0.17%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9a872a...f957110. Read the comment docs.

@ChenZhao44
Copy link
Member Author

@Roger-luo it's ready to be merged now.

@Roger-luo
Copy link
Member

can we chat about this in the next meeting? I'll merge it after that.

@ChenZhao44
Copy link
Member Author

@Roger-luo CI failed because the SWAP PR of YaoHIR is not merged.

@ChenZhao44
Copy link
Member Author

@Roger-luo Naive simplification methods (e.g. simplify_swap!) for Chain are temporarily in this package. We need to make a new package for them later.

@Roger-luo
Copy link
Member

Yeah I think there are a few better ways of doing SWAP simplification. let's do that in the exact match package later.

src/simplify.jl Outdated
g = pop!(qc.args)
if g isa Gate
if g.operation === SWAP
loc1, loc2 = g.locations.storage[1:2]
Copy link
Member

Choose a reason for hiding this comment

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

use plain here to get the plain old data

continue
end
end
pushfirst!(chain_after_swap.args, map_locations(loc_map, g))
Copy link
Member

Choose a reason for hiding this comment

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

I feel SWAP should be simplified by looking at qubit dependency etc. but let's prob go with this simple one first.

src/simplify.jl Outdated
j += 1
g isa Ctrl || continue
if g.gate === X && length(g.gate.locations) == 1 && length(g.ctrl) == 1
loc = g.gate.locations.storage[]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
loc = g.gate.locations.storage[]
loc = plain(g.gate.locations)[]

src/simplify.jl Outdated
j += 1
g isa Ctrl || continue
if g.gate === X && length(g.gate.locations) == 1 && length(g.ctrl) == 1
loc = g.gate.locations.storage[]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
loc = g.gate.locations.storage[]
loc = plain(g.gate.locations)[]

src/simplify.jl Outdated
g isa Ctrl || continue
if g.gate === X && length(g.gate.locations) == 1 && length(g.ctrl) == 1
loc = g.gate.locations.storage[]
ctrl = g.ctrl.storage.storage[]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ctrl = g.ctrl.storage.storage[]
ctrl = plain(g.ctrl.storage)[]

src/simplify.jl Outdated
g = qc_after_swap[j]
j += 1
g isa Ctrl || continue
if g.gate === X && length(g.gate.locations) == 1 && length(g.ctrl) == 1
Copy link
Member

@Roger-luo Roger-luo May 13, 2021

Choose a reason for hiding this comment

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

if you use MLStyle, this can just be

@switch gate begin
    @case Ctrl(Gate(X, locs::Locations), ctrl::Locations)
          length(locs) == 1 && length(ctrl) == 1 || continue # or we should use a Guard pattern here
          plain_loc = plain(locs)[]
          plain_ctrl = plain(ctrl)[]
    @case _ # other case
end
     

Copy link
Member

Choose a reason for hiding this comment

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

this pattern asserts the location type since in HIR we allow locations to be a SSAValue

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried but it seems it doesn't work well because we have a break here.

src/simplify.jl Outdated
loc = g.gate.locations.storage[]
ctrl = g.ctrl.storage.storage[]
if haskey(qmap, loc) && haskey(qmap, ctrl)
insert!(qc, j, convert_to_gate(Val(:CNOT), ctrl, loc))
Copy link
Member

Choose a reason for hiding this comment

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

I'm actually thinking maybe should just use the intrinsic type and pattern match in convert_to_gate function too.

@@ -1,3 +1,46 @@
using YaoHIR: X, Z, S, T, SWAP, Rz, Rx, shift

convert_to_gate(::Val{:X}, loc) = Gate(X, Locations(loc))
Copy link
Member

Choose a reason for hiding this comment

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

if we just XGate type over Val{:X} we can prob get rid of these functions in this package.

Copy link
Member Author

Choose a reason for hiding this comment

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

It will simplify for most gates except for CNOT and CZ. For consistency, make it's better to use the old one.

r0 += i - 1
r0 == i && break
M[i,:] = M[i,:] .⊻ M[r0,:]
step = GEStep(:addto, r0, i)
M_r0 = M[r0,:]
Copy link
Member

Choose a reason for hiding this comment

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

maybe should use @views?

Copy link
Member Author

Choose a reason for hiding this comment

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

This will swap two rows. So we have to record one of the rows. Hence, @view is not the right way.

@Roger-luo Roger-luo closed this May 13, 2021
@Roger-luo Roger-luo reopened this May 13, 2021
@Roger-luo Roger-luo merged commit 75f9591 into master May 14, 2021
@Roger-luo Roger-luo deleted the extract-opt branch May 14, 2021 17:11
@ChenZhao44 ChenZhao44 linked an issue May 15, 2021 that may be closed by this pull request
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.

Optimize the circuit extraction algorithm
2 participants