Opened 4 years ago

Last modified 4 years ago

#25896 new defect

DuadicCodeEvenPair/DuadicCodeOddPair are broken

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.4
Component: coding theory Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/chapoton/25379 Commit: ea8fa5c491d9055ed1b213526d439ce82177de34
Dependencies: Stopgaps: #25379

Status badges

Description (last modified by jdemeyer)

  1. The use of _lift2smallest_field is very dubious:
    sage: S1 = [1,3,4,5,7,12,13]; S2 = [2,6,8,9,10,11,14]
    sage: from sage.coding.code_constructions import _is_a_splitting
    sage: _is_a_splitting(S1, S2, 15)
    True
    sage: codes.DuadicCodeEvenPair(GF(7), S1, S2)
    ...
    ValueError: 5*z + 2 is not in the image of (map internal to coercion system -- copy before use)
    Ring morphism:
      From: Finite Field of size 7
      To:   Finite Field in z of size 7^2
    
  1. It doesn't work as documented:
    sage: S1 = [1,3,4,5,7,12,13]; S2 = [2,6,8,9,10,11,14]
    sage: from sage.coding.code_constructions import _is_a_splitting
    sage: _is_a_splitting(S1, S2, 15)
    True
    sage: codes.DuadicCodeEvenPair(GF(3), S1, S2)
    ---------------------------------------------------------------------------
    ArithmeticError                           Traceback (most recent call last)
    <ipython-input-27-0b1b7f661976> in <module>()
    ----> 1 codes.DuadicCodeEvenPair(GF(Integer(3)), S1, S2)
    
    /usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/coding/code_constructions.pyc in DuadicCodeEvenPair(F, S1, S2)
        359         raise TypeError("%s, %s must be a splitting of %s."%(S1,S2,n))
        360     q = F.order()
    --> 361     k = Mod(q,n).multiplicative_order()
        362     FF = GF(q**k,"z")
        363     z = FF.gen()
    
    /usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/rings/finite_rings/integer_mod.pyx in sage.rings.finite_rings.integer_mod.IntegerMod_abstract.multiplicative_order (build/cythonized/sage/rings/finite_rings/integer_mod.c:22221)()
       1766             return sage.rings.integer.Integer(self.__pari__().znorder())
       1767         except PariError:
    -> 1768             raise ArithmeticError("multiplicative order of %s not defined since it is not a unit modulo %s"%(
       1769                 self, self.__modulus.sageInteger))
       1770 
    
    ArithmeticError: multiplicative order of 3 not defined since it is not a unit modulo 15
    
  1. The code assumes implicitly that the generator of a finite field is a multiplicative generator (this is easy to fix).
  1. Since you explicitly refer to the docstring of _is_a_splitting, the latter should be public and documented.

I can imagine that 1. and 2. should be considered "bad input" but that's not clear. It seems that there is an additional hidden assumption that q S1 = S1 (and then also q S2 = S2).

Updating the documentation is the absolute minimum that must be done before anything else: if we don't even know what the function should do, we cannot fix it.

Change History (8)

comment:1 Changed 4 years ago by jdemeyer

  • Branch set to u/chapoton/25379
  • Commit set to ea8fa5c491d9055ed1b213526d439ce82177de34

Just adding the branch for reference, it probably isn't a good solution.


New commits:

ea8fa5cenhance one lifting procedure in code constructions

comment:2 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 4 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.