Opened 10 years ago

Last modified 3 years ago

#11777 needs_work defect

Coercion/printing problem with p-adics

Reported by: robharron Owned by: roed
Priority: minor Milestone: sage-6.4
Component: padics Keywords: padics, repr_spec, repr_gen, rd2, padicIMA
Cc: roed Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

A strange problem maybe cause by a coercion problem when coercing from a number field into a p-adic field. Upon printing, repr_spec (reps. repr_gen if in 'bars' mode) ends up receiving an element whose call _ext_p_list(True) returns an empty list which causes some errors to be raised. Reproducing source code below.

Attachments (1)

11777-ver2.patch (5.3 KB) - added by justin 9 years ago.
Updated patch, to be applied instead of the previous one.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 10 years ago by robharron

The following code:

p = 53
K.<zeta12> = CyclotomicField(12)
QQp = Qp(p, prec=1000000)
print QQp.has_root_of_unity(3)
QQq = Qq(p^2, names='z', prec=1000000)
print QQq.has_root_of_unity(3)
print QQq(zeta12)
print z^2
aaa=QQq(zeta12^2)
print aaa._ext_p_list(True)
print aaa

produces the output:

False
True
z + O(53^1000000)
(4*z + 51) + (52*z + 52)*53 + (52*z + 52)*53^2 + (52*z + 52)*53^3 + (52*z + 52)*53^4 + (52*z + 52)*53^5 + (52*z + 52)*53^6 + (52*z + 52)*53^7 + (52*z + 52)*53^8 + (52*z + 52)*53^9 + (52*z + 52)*53^10 + (52*z + 52)*53^11 + (52*z + 52)*53^12 + (52*z + 52)*53^13 + (52*z + 52)*53^14 + (52*z + 52)*53^15 + (52*z + 52)*53^16 + (52*z + 52)*53^17 + (52*z + 52)*53^18 + (52*z + 52)*53^19 + O(53^20)
[]
Traceback (most recent call last):    print QQq(zeta12)
  File "", line 1, in <module>
    
  File "/private/var/folders/c+/c+SfSVUQGdqrYbVIA1yDLE+++TI/-Tmp-/tmpiPpw1C/___code___.py", line 13, in <module>
    exec compile(u'print aaa
  File "", line 1, in <module>
    
  File "sage_object.pyx", line 154, in sage.structure.sage_object.SageObject.__repr__ (sage/structure/sage_object.c:1463)
  File "padic_generic_element.pyx", line 429, in sage.rings.padics.padic_generic_element.pAdicGenericElement._repr_ (sage/rings/padics/padic_generic_element.c:5616)
  File "padic_printing.pyx", line 848, in sage.rings.padics.padic_printing.pAdicPrinter_class.repr_gen (sage/rings/padics/padic_printing.cpp:9279)
  File "padic_printing.pyx", line 952, in sage.rings.padics.padic_printing.pAdicPrinter_class._repr_gen (sage/rings/padics/padic_printing.cpp:10957)
  File "padic_printing.pyx", line 1092, in sage.rings.padics.padic_printing.pAdicPrinter_class._repr_spec (sage/rings/padics/padic_printing.cpp:13067)
RuntimeError: repr_spec called on zero

It seems like there's a problem coercing zeta12^2, since it should give z^2 (note that if using 'bars' as print_mode, repr_gen raises an index out of bounds error as it attempts to access index 0 of aaa._ext_p_list(True). I don't know exactly what's going on, but at the very least shouldn't is_inexact_zero test for ._ext_p_list(True) returning an empty list?

comment:2 Changed 10 years ago by robharron

Wait, the coercion from K to QQq is just sending K.gen() to QQq.gen(). That shouldn't be happening.

comment:3 Changed 10 years ago by roed

  • Status changed from new to needs_review

The fallback mode for the __init__ method of padic_ZZ_pX_CR_element is to try to convert the input to a list. This is why the conversion (not coercion: you explicitly asked Sage to construct an element of QQq if possible) is just sending generator to generator.

The empty _ext_p_list is the result of a missing check in the method constructing a p-adic element from a ZZ_pX. I've added that check in the attached patch.

The coercion map you want from K to QQq needs to wait until completions of number fields are supported: see http://wiki.sagemath.org/padics/Completions.

Thanks for catching the bug!

comment:4 Changed 9 years ago by johanbosman

  • Status changed from needs_review to needs_work

I think the message would be even clearer if it also said *what* exactly isn't implemented. ;).

comment:5 Changed 9 years ago by roed

  • Status changed from needs_work to needs_review

I added the message "The degree of the NTL polynomial must be at most the degree of the modulus" to each NotImplementedError.

Changed 9 years ago by justin

Updated patch, to be applied instead of the previous one.

comment:6 Changed 9 years ago by justin

I looked at the patch (11777.patch). It seems fine except that the exception message says that the polynomial degree must be at most that of the modulus; the test actually wants "less than".

I've attached 11777-ver2.patch, which should be used instead of the original.

comment:7 Changed 9 years ago by justin

Oops. The "ver2" patch should be applied *in addition to and after* the original patch.

My bad.

comment:8 Changed 9 years ago by roed

Justin's patch looks good to me.

comment:9 Changed 9 years ago by jen

  • Status changed from needs_review to positive_review

comment:10 Changed 9 years ago by jen

  • Keywords rd2 added

comment:11 Changed 9 years ago by jdemeyer

This needs to be rebased to http://boxen.math.washington.edu/home/release/sage-5.0.beta9/ (to be released very soon)

comment:12 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:13 Changed 9 years ago by justin

Ah, I wouldn't say the patch needs to be rebased. It appears to be more that this bug needs to be closed as "no longer applicable". David? If this is the case, there are a few references, in comments, to NTL; are these still valid?

comment:14 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:15 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:16 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:17 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:18 Changed 3 years ago by roed

  • Keywords padicIMA added
Note: See TracTickets for help on using tickets.