Feat/rotation_z gate #362
Feat/rotation_z gate #362
Conversation
fSosaRey
temporarily deployed
to
thunderhead-dev
GitHub Actions
Inactive
| @@ -1838,5 +1838,5 @@ function compare_operators(H_0::AbstractOperator, H_1::AbstractOperator)::Bool | |||
| δ = get_canonical_global_phase(m_0) - get_canonical_global_phase(m_1) | |||
|
|
|||
| # We need to check atol for values close to zero | |||
| return isapprox(m_0, exp(im * δ) .* m_1; atol = sqrt(eps(typeof(δ)))) | |||
| return isapprox(m_0, exp(im * δ) .* m_1; atol = 1e-6) | |||
There was a problem hiding this comment.
I changed this because between rotation_z and it's universal equivalent there was an error 20% above sqrt(eps()), 1.4e-8 vs 1.2e-8, making the tests fail
There was a problem hiding this comment.
Would this be fixed by also specifying the rtol instead of increasing the atol? According to the Julia documentation, rtol is zeroed instead of being sqrt(eps(typeof(\delta))), which is not what I originally intended.
There was a problem hiding this comment.
The following is the failing test case. I tried using rtol instead of atol but it also needs to be raised to at least 2*sqrt(eps(typeof(delta)).
using Snowflurry
using Test
target = 1
theta = pi / 5
instr = rotation_z(target, theta)
universal_equivalent =
Snowflurry.as_universal_gate(target, get_operator(get_gate_symbol(instr)))
alpha = Snowflurry.get_canonical_global_phase(get_matrix(get_operator(get_gate_symbol(instr))))
println("alpha: $alpha")
println("instr: $(exp(im*-alpha) * get_operator(get_gate_symbol(instr)))")
println("universal: $(get_operator(get_gate_symbol(universal_equivalent)))")
@test Snowflurry.compare_operators(
get_operator(get_gate_symbol(instr)),
get_operator(get_gate_symbol(universal_equivalent)),
)
There was a problem hiding this comment.
I see we are compounding errors, and then the tolerances stop making sense. For now, we should leave it as is. I've created a very-low-priority issue to allow specifying the tolerances of compare functions here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 2467 2471 +4
=========================================
+ Hits 2467 2471 +4 ☔ View full report in Codecov by Sentry. |
fSosaRey
temporarily deployed
to
thunderhead-dev
GitHub Actions
Inactive
fSosaRey
temporarily deployed
to
thunderhead-dev
GitHub Actions
Inactive
| @@ -1838,5 +1838,5 @@ function compare_operators(H_0::AbstractOperator, H_1::AbstractOperator)::Bool | |||
| δ = get_canonical_global_phase(m_0) - get_canonical_global_phase(m_1) | |||
|
|
|||
| # We need to check atol for values close to zero | |||
| return isapprox(m_0, exp(im * δ) .* m_1; atol = sqrt(eps(typeof(δ)))) | |||
| return isapprox(m_0, exp(im * δ) .* m_1; atol = 1e-6) | |||
There was a problem hiding this comment.
Would this be fixed by also specifying the rtol instead of increasing the atol? According to the Julia documentation, rtol is zeroed instead of being sqrt(eps(typeof(\delta))), which is not what I originally intended.
src/core/quantum_gate.jl
Outdated
| """ | ||
| rotation_z(target, lambda) | ||
|
|
||
| Return a `Gate` that applies a symmetric phase shift of `-lambda/2` and `+lambda/2` to the |
There was a problem hiding this comment.
See discussion on #361 regarding "symmetric phase shift".
There was a problem hiding this comment.
updated to a more standard docstring
fSosaRey
force-pushed
the
feat/RotationZ-gate
branch
from
2e5725c
to
d7360c2
Compare
fSosaRey
temporarily deployed
to
thunderhead-dev
GitHub Actions
Inactive
fSosaRey
temporarily deployed
to
thunderhead-dev
GitHub Actions
Inactive
No description provided.