sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 481 forks source link

Coercion/printing problem with p-adics #11777

Open b11c8a96-399b-45f8-a2be-ae7169948853 opened 13 years ago

b11c8a96-399b-45f8-a2be-ae7169948853 commented 13 years ago

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.

CC: @roed314

Component: padics

Keywords: repr_spec, repr_gen, rd2, padicIMA

Issue created by migration from https://trac.sagemath.org/ticket/11777

b11c8a96-399b-45f8-a2be-ae7169948853 commented 13 years ago
comment:1

The following code:

p = 53 K. = 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?

b11c8a96-399b-45f8-a2be-ae7169948853 commented 13 years ago
comment:2

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

roed314 commented 13 years ago
comment:3

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!

1659f18b-8e7f-4ace-87e0-ea435f3ce618 commented 12 years ago
comment:4

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

roed314 commented 12 years ago
comment:5

I added the message "The degree of the NTL polynomial must be at most the degree of the modulus" to each NotImplementedError. Test edit over and over again

667aa960-5c62-4445-bf8d-25c93fb0a27b commented 12 years ago

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

667aa960-5c62-4445-bf8d-25c93fb0a27b commented 12 years ago
comment:6

Attachment: 11777-ver2.patch.gz

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.

667aa960-5c62-4445-bf8d-25c93fb0a27b commented 12 years ago
comment:7

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

My bad.

roed314 commented 12 years ago
comment:8

Justin's patch looks good to me.

jbalakrishnan commented 12 years ago

Changed keywords from repr_spec, repr_gen to repr_spec, repr_gen, rd2

jdemeyer commented 12 years ago
comment:11

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

667aa960-5c62-4445-bf8d-25c93fb0a27b commented 12 years ago
comment:13

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?

roed314 commented 6 years ago

Changed keywords from repr_spec, repr_gen, rd2 to repr_spec, repr_gen, rd2, padicIMA

mkoeppe commented 1 year ago
comment:19

Removed long-closed milestone.