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: |
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)
Change History (19)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
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
- 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
- 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
- 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
.
comment:6 Changed 9 years ago by
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
Oops. The "ver2" patch should be applied *in addition to and after* the original patch.
My bad.
comment:8 Changed 9 years ago by
Justin's patch looks good to me.
comment:9 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 9 years ago by
- Keywords rd2 added
comment:11 Changed 9 years ago by
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
- Status changed from positive_review to needs_work
comment:13 Changed 9 years ago by
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
- Milestone changed from sage-5.11 to sage-5.12
comment:15 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:16 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:17 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:18 Changed 3 years ago by
- Keywords padicIMA added
The following code:
produces the output:
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?