Skip to content

Conversation

@Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented Jun 26, 2025

The classical LiftedProduct code parity checks matrices have not really been used as seed for any quantum code so this bug was not detected. Reference: #356

Edit:

🐞 Stacktrace of MethodError when calling parity_checks

ERROR: MethodError: no method matching lift(::typeof(representation_matrix), ::Matrix{GroupAlgebraElem{FqFieldElem, GroupAlgebra{…}}})
The function `lift` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  lift(::fqPolyRepPolyRingElem, ::Any)
   @ Hecke ~/.julia/packages/Hecke/RwzTY/src/LocalField/Poly.jl:117
  lift(::ZZRing, ::zzModRingElem)
   @ Nemo ~/.julia/packages/Nemo/mOOnX/src/flint/nmod.jl:35
  lift(::ZZRing, ::ZZModRingElem)
   @ Nemo ~/.julia/packages/Nemo/mOOnX/src/flint/fmpz_mod.jl:35
  ...
Stacktrace:
 [1] parity_checks(c::QuantumCliffordHeckeExt.LiftedCode)
   @ QuantumCliffordHeckeExt ~/Desktop/QuantumClifford.jl/ext/QuantumCliffordHeckeExt/lifted.jl:107
 [2] parity_matrix(c::QuantumCliffordHeckeExt.LiftedCode)
   @ QuantumClifford.ECC ~/Desktop/QuantumClifford.jl/src/ecc/ECC.jl:148
 [3] top-level scope
   @ REPL[100]:1
Some type information was truncated. Use `show(err)` to see complete types.

Additionally, I have not added doctest for this first wrapper constructor that calls the second constructor and I am not sure if there is any potential bug calling this or not.

function LiftedCode(A::Matrix{GroupAlgebraElem{FqFieldElem, <: GroupAlgebra}}; GA::GroupAlgebra=parent(A[1,1]))

  • 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.
  • We recently started enforcing formatting checks. If formatting issues are reported in the new code you have written, please correct them.

@Fe-r-oz Fe-r-oz changed the title fix generation of classical LiftedCode and correct parameters calculation fix parity matrix generation of classical LiftedCode and correct parameters calculation Jun 26, 2025
@Fe-r-oz Fe-r-oz force-pushed the fa/fixclassicallpcode branch 2 times, most recently from 0ee85fd to bc58e05 Compare June 26, 2025 09:59
@Fe-r-oz Fe-r-oz force-pushed the fa/fixclassicallpcode branch from bc58e05 to 4cc8eac Compare June 26, 2025 10:02
@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.16%. Comparing base (cd76712) to head (c8c7230).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #525      +/-   ##
==========================================
+ Coverage   85.11%   85.16%   +0.05%     
==========================================
  Files         121      121              
  Lines        7248     7248              
==========================================
+ Hits         6169     6173       +4     
+ Misses       1079     1075       -4     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Fe-r-oz Fe-r-oz marked this pull request as ready for review June 26, 2025 13:59
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 26, 2025

This PR is ready for review, Thank you!

@Fe-r-oz Fe-r-oz requested a review from Krastanov June 26, 2025 23:15
@Fe-r-oz Fe-r-oz changed the title fix parity matrix generation of classical LiftedCode and correct parameters calculation fix: parity matrix generation of classical LiftedCode and correct parameters calculation Jun 28, 2025
Copy link
Member

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

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

thanks, this is great! Could you add a small test for what used to fail, just to make sure that similar failures do not get reintroduced at some point.

@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Jun 29, 2025

Thank you for your feedback! I have added basic tests with tests_nowarn that just check there is no method error calling stuff that resulted in method errors, I will check literature about properties on these classical codes. I have placed the tests in test_ecc_throws.jl.

The stacktrace of the errors resulting from code_s, code_n and the suggested fix:

julia> import Hecke; import QuantumClifford.ECC: LiftedCode, code_n, code_k, code_s
julia> import QuantumClifford.ECC.QECCore: parity_matrix
julia>  base_matrix = [0 0 0 0; 0 1 2 5; 0 6 3 1]; l = 3;
julia> c LiftedCode(base_matrix, l);
julia> size(zero(c.GA), 2 or 1) # currently, we use this in code_n or code_s that throw error
ERROR: MethodError: no method matching size(::GroupAlgebraElem{FqFieldElem, GroupAlgebra{FqFieldElem, FinGenAbGroup, FinGenAbGroupElem}}, ::Int64)
The function `size` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  size(::BitVector, ::Integer)
   @ Base bitarray.jl:107
  size(::Hecke.ZZRingElem_Array_Mod.ZZRingElem_Array, ::Int64)
   @ Hecke ~/.julia/packages/Hecke/RwzTY/src/Sparse/ZZRow.jl:65
  size(::Type{SIMD.Vec{N, T}}, ::Integer) where {N, T}
   @ SIMD ~/.julia/packages/SIMD/hyXY3/src/simdvec.jl:74
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[30]:1

julia> size(c.repr(zero(c.GA)), 2)
3

julia> size(c.A, 2) * order(group(c.GA))
12
julia> size(c.A, 1) * order(group(c.GA)) 
9

julia> size(c.A, 2) * size(c.repr(zero(c.GA)), 2)
12
julia> size(c.A, 1) * size(c.repr(zero(c.GA)), 1)
9

Defined code_k for the classical code as $k = n - rank(H)$

@Fe-r-oz Fe-r-oz changed the title fix: parity matrix generation of classical LiftedCode and correct parameters calculation fix parity_matrix generation and parameter calculations in classical LiftedCode Jul 5, 2025
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Aug 9, 2025

ping @Krastanov

@Fe-r-oz Fe-r-oz added the Skip-Changelog label for control of CI: skips the changelog check label Oct 28, 2025
@Fe-r-oz Fe-r-oz mentioned this pull request Oct 30, 2025
5 tasks
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Oct 31, 2025

@Krastanov, when you have a moment, please let me know your feedback. I have fixed the LiftedCode constructor, added it to the CECC base, and completed the code review suggestions. Please let me know if it's ready to merge. The test failures are unrelated to this PR. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip-Changelog label for control of CI: skips the changelog check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants