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

Some feedbacks #49

Open
1 of 6 tasks
GiggleLiu opened this issue Aug 29, 2020 · 1 comment
Open
1 of 6 tasks

Some feedbacks #49

GiggleLiu opened this issue Aug 29, 2020 · 1 comment

Comments

@GiggleLiu
Copy link
Member

GiggleLiu commented Aug 29, 2020

  • simplify!(Rule{:lc}(), ::ZXDiagram) should not work, but it does not give an error.
  • the _ctrl in push_ctrl_gate! looks not necessary.
  • the naming of simplify! and replace! are not intuitive, should be something like simplify_recursive! and simplify_once!?
  • it would be nice to have a pipeline interface zxg |> srule!(:lc) |> srule!(:p1) |> srule_once!(:pab)
  • the following code looks confusing to me
  • do you mind changing name of phase to e.g. getphase`, because it conflicts with the phase gate in Yao.
julia> zxd = ZXDiagram(4);

julia> push_gate!(zxd, Val(:Z), 4);

julia> push_gate!(zxd, Val(:X), 4)
ZX-diagram with 10 vertices and 6 multiple edges:
(S_1{input} <-1-> S_2{output})
(S_3{input} <-1-> S_4{output})
(S_5{input} <-1-> S_6{output})
(S_7{input} <-1-> S_9{phase = 0//1π})
(S_8{output} <-1-> S_10{phase = 0//1π})
(S_9{phase = 0//1π} <-1-> S_10{phase = 0//1π})

X gate is push_gate!(zxd, Val(:X), 4, 0//1), Z gate is push_gate!(zxd, Val(:Z), 4, π)
Maybe just forbid this interface?

@Roger-luo
Copy link
Member

simplify!(Rule{:lc}(), ::ZXDiagram) should not work, but it does not give an error.

@ChenZhao44 define the method as:

simplify!(Rule{:lc}(), ::ZXDiagram) = error("cannot perform lc rule on ZX diagram")

would solve the issue, since simplify! implements the generic one.

the _ctrl in push_ctrl_gate! looks not necessary.

yeah, we need to rename this at some point.

the naming of simplify! and replace! are not intuitive, should be something like simplify_recursive! and simplify_once!?

simplify! and replace! has its convention in general symbolic computation/term rewrite packages cross Julia and literature. simpliy always mean recursively rewrite, and replace means substitution (or it's called substitution). But I think the doc string is very vague @ChenZhao44 we should improve doc string to mention this difference and mention something like

See also [`simplify!`](@ref).

On the other hand, we will need to integrate with SymbolicUtils at one point, since they will provide some graph match implementations, and we can serve as an simplification/contraction engine for other packages later.

it would be nice to have a pipeline interface zxg |> srule!(:lc) |> srule!(:p1) |> srule_once!(:pab)

I think just letting simplify! and replace! to have a curried version is sufficient. Also note, we don't actually have much use case for single pass simplification. The simplification algorithm always have a list of rules to apply.

We should have a list-like simplification interface at some point, tho. current methods like clifford_simplification is not necessary actually, this also as a result to have duplicated code in phase_teleport. But I suggest to postpone this to polish later.

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

No branches or pull requests

2 participants