fix #191: too restrictive typeassert for MixedDestabilizer

[Fe-r-oz]

This now works and all the test passes as well:

julia> random_destabilizer(5,6) |> stabilizerview |> MixedDestabilizer
𝒟ℯ𝓈𝓉𝒶𝒷━━
+ Z_____
+ _Z____
+ __Z___
+ ___Z__
+ _____X
𝒳ₗ━━━━━━
+ ZZZZX_
𝒮𝓉𝒶𝒷━━━━
+ XZ_ZZ_
- ZX__Z_
+ __YZZ_
+ Z_ZXZ_
+ _____Z
𝒵ₗ━━━━━━
+ ____Z_

The JET errors does not increase as well. Hopefully, this is plausible.

• The code is properly formatted and commented.
• Substantial new functionality is documented within the docs.
• All new functionality is tested.
• All of the automated tests on github pass.

[Fe-r-oz]

The PR is ready for review. Thank you!

P.S. My bad for the rebase. Initially, I had used : after 191 in commit message which didn't pushed the issue to appear in Development. However, I noticed simply using #issue can make the issue appear in Development. So, I did the rebase using reword to remove : in hash191: . But the issue still didn't appear in Development...

[Krastanov]

Could you elaborate why this is necessary? Maybe add tests that would have failed without this change?

[Krastanov]

The proper fix for this probably requires better parameterization of the input argument. The issue is that the following T is not what is preserved:

MixedDestabilizer(::Stabilizer{T}) where {T}

Rather what is preserved is the T in here:

MixedDestabilizer(::Stabilizer{Tableau{P, <:AbstractMatrix{T}}}) where {P, T}

Maybe we should define eltype for these guys.

eltype(::Stabilizer{Tableau{P, <:AbstractMatrix{T}}}) where {P, T} = T

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.97%. Comparing base (f3eb7cd) to head (0122d72).
Report is 7 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #366      +/-   ##
==========================================
+ Coverage   82.42%   83.97%   +1.55%     
==========================================
  Files          70       70              
  Lines        4580     4918     +338     
==========================================
+ Hits         3775     4130     +355     
+ Misses        805      788      -17     

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

[Fe-r-oz]

I attempted the suggested approach by defining the following method:

Base.eltype(::Stabilizer{Tableau{P, <:AbstractMatrix{T}}}) where {P, T} = T

Afterward, I initialized a variable t using:

t = zeros(eltype(stab), n * 2, n)

However, when executing random_destabilizer(5, 6) |> stabilizerview |> MixedDestabilizer, I encountered the following warning and error messages:

INFO: Given QuantumClifford was explicitly requested, output will be shown live.
WARNING: Method definition eltype(QuantumClifford.Stabilizer{QuantumClifford.Tableau{P, var"#s37"} where var"#s37" <: AbstractArray{T, 2}}) where {P, T} in module QuantumClifford at /home/Desktop/191/QuantumClifford.jl/src/QuantumClifford.jl:408 overwritten at /home/Desktop/191/QuantumClifford.jl/src/QuantumClifford.jl:940.
ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.

I have followed your suggestions from #366 (review) by including a comment to elaborate the changes.

I hope this is reasonable.

[Fe-r-oz]

Maybe add tests that would have failed without this change?

Thank you! I have added tests that fail on the master branch, which reproduce the error without the need for fancy expressions like random_destabilizer(5, 6) |> stabilizerview |> MixedDestabilizer.

ERROR: TypeError: in typeassert, expected QuantumClifford.Tableau{SubArray{UInt8, 1, Vector{UInt8}, Tuple{UnitRange{Int64}}, true}, SubArray{UInt64, 2, Matrix{UInt64}, Tuple{Base.Slice{Base.OneTo{Int64}}, UnitRange{Int64}}, true}}, got a value of type QuantumClifford.Tableau{Vector{UInt8}, Matrix{UInt64}}
Stacktrace:
 [1] MixedDestabilizer(stab::Stabilizer{QuantumClifford.Tableau{SubArray{…}, SubArray{…}}}; undoperm::Bool, reportperm::Bool)
   @ QuantumClifford ~/Desktop/hnEW/QuantumClifford.jl/src/QuantumClifford.jl:574
 [2] top-level scope

Consequently, the suggestions from #366 (review) have been fully incorporated. Thus, this PR resolves #191

• comment elaborating the change
• tests that would have failed (on master branch) without this change

[Fe-r-oz]

Yup, the JET and compiler have improved as previously removing ::T caused some tests to fail.

Thank you for the simplification.

[Krastanov]

Great! Thanks for the fix!