Opened 11 years ago
Closed 4 years ago
#8240 closed defect (fixed)
cannot coerce padic field into residue field
Reported by:  dmharvey  Owned by:  roed 

Priority:  major  Milestone:  sage8.1 
Component:  padics  Keywords:  
Cc:  roed, robertwb, saraedum, caruso, kedlaya  Merged in:  
Authors:  David Roe  Reviewers:  Kiran Kedlaya 
Report Upstream:  N/A  Work issues:  
Branch:  1fc48d1 (Commits, GitHub, GitLab)  Commit:  1fc48d1a6f4baf9625c291fdb8eae0f8d1109d3c 
Dependencies:  #14825  Stopgaps: 
Description
sage: K.<a> = Qq(25) sage: F = K.residue_field() sage: F(a) Traceback (click to the left of this block for traceback) ... TypeError: unable to coerce
Perhaps this is a "feature request", but it seems like a pretty basic feature...
(It works fine for prime fields)
Attachments (4)
Change History (47)
comment:1 Changed 11 years ago by
 Cc robertwb added
Changed 9 years ago by
comment:2 Changed 9 years ago by
 Dependencies set to #12062, #12053, #12064, #12077
I've updated the dependencies. If you care about this ticket, review them!
Note that the 8240.patch is a duplicate of #12077 and should be removed. 8240_ext1.patch and 8240_ext2.patch are in progress: I basically didn't finish writing them. Feel free to add to these patches in the same vein as #12077.
comment:3 Changed 4 years ago by
 Branch set to u/roed/residue_coercion
comment:4 Changed 4 years ago by
 Commit set to c456a748154ba2352e8475b89a5bdd2a56973dc6
Branch pushed to git repo; I updated commit sha1. New commits:
c456a74  Adding lots of documentation, fixing doctests and problems

comment:5 Changed 4 years ago by
 Dependencies #12062, #12053, #12064, #12077 deleted
 Work issues set to resolve doctest errors from incorrect reductions elsewhere in Sage
Last 10 new commits:
bd15d71  add exact keyword

99dccf6  Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision

ef4ed33  Fix SEEALSO:

1754b44  Fix exact=True errors in the right branch

a29bf7a  Merge branch 't/20310/change_precision' into t/23471/padic_ext_conversion

f985aa0  Add ability to call with arguments

9763d7a  Fix implementation of _call_with_args

3b52f2c  Fix bug from args being a tuple, not a list

1944974  Add morphisms to and from residue field (still need doctests)

c456a74  Adding lots of documentation, fixing doctests and problems

comment:6 Changed 4 years ago by
sage t warnlong 53.3 src/sage/schemes/hyperelliptic_curves/hyperelliptic_padic_field.py # 27 doctests failed sage t warnlong 53.3 src/sage/modular/btquotients/pautomorphicform.py # 24 doctests failed sage t warnlong 53.3 src/sage/schemes/elliptic_curves/padics.py # 1 doctest failed sage t warnlong 53.3 src/sage/modular/btquotients/btquotient.py # 4 doctests failed sage t warnlong 53.3 src/sage/schemes/hyperelliptic_curves/monsky_washnitzer.py # 4 doctests failed
comment:7 Changed 4 years ago by
 Commit changed from c456a748154ba2352e8475b89a5bdd2a56973dc6 to 550a831d92fd7055389ab98bcf9ea3c3e57f272f
Branch pushed to git repo; I updated commit sha1. New commits:
550a831  Merge branch 'u/roed/residue_coercion' of git://trac.sagemath.org/sage into t/8240/residue_coercion

comment:8 Changed 4 years ago by
 Cc saraedum caruso kedlaya added
So, there are a number of failures coming from following change
sage: R = Zp(5); S = Zmod(5^5) sage: a = R(1, 3); a 1 + O(5^3) sage: S(a) Traceback (most recent call last): ... PrecisionError: Not enough precision to determine reduction.
Previously, Sage performed this reduction without complaint. Some questions:
 Do we want a coercion from
R
toS
? Mathematically, this coercion is very similar to the one fromZZ
toZmod(N)
, so I think the answer is yes.  If there is a coercion, what should its behavior be on elements without enough precision, as in the example above? A
PrecisionError
seems reasonable, but normally coercions don't raise errors....  The coercion model demands that if there is a coercion between two parents, then that morphism should be used as the conversion as well (this is implemented in
discover_convert_map
, which calls_internal_coerce_map_from
as the first step). So we can't have different behavior for the coercion and the conversion fromR
toS
.  It's pretty annoying to have these kind of
PrecisionError
s in conversions. For example, conversions are used inchange_ring
on polynomials, and there are plenty of examples in the Sage library wherechange_ring
that trigger this kind ofPrecisionError
.
Any suggestions?
comment:9 Changed 4 years ago by
I just checked the following:
sage: R1 = RealIntervalField(10) sage: C1 = ComplexIntervalField(10) sage: C1.coerce_map_from(R1) Call morphism: From: Real Interval Field with 10 bits of precision To: Complex Interval Field with 10 bits of precision sage: C1 = ComplexIntervalField(11) sage: C1.coerce_map_from(R1) sage: C1.convert_map_from(R1) Call morphism: From: Real Interval Field with 10 bits of precision To: Complex Interval Field with 11 bits of precision
So, there's a precedent in the archimedean code to only have a convert map, not a coerce map, when mapping from a lower precision to a higher precision. For Zp(5) to Zmod(5^{5}), you're mapping an inexact ring to an exact ring, so this would suggest that there should be no coercion, only a conversion.
comment:10 followup: ↓ 11 Changed 4 years ago by
Yeah, just having a conversion seems like the simplest solution. I think it may be reasonable to have a coercion to the residue field though?
comment:11 in reply to: ↑ 10 Changed 4 years ago by
Replying to roed:
Yeah, just having a conversion seems like the simplest solution. I think it may be reasonable to have a coercion to the residue field though?
Yeah, I think coercion to the residue field makes sense, though a PrecisionError? will still need to be raised for O(5^{0}); I think that's reasonable since O(5^{0}) basically means you have no idea what element of Zp you have!
comment:12 Changed 4 years ago by
 Commit changed from 550a831d92fd7055389ab98bcf9ea3c3e57f272f to c08c76432d957b62d799581051cbab40c9766de2
comment:13 Changed 4 years ago by
 Status changed from new to needs_review
 Work issues resolve doctest errors from incorrect reductions elsewhere in Sage deleted
comment:14 followups: ↓ 15 ↓ 16 Changed 4 years ago by
IMO, It's a kinda weird to promote reduction modulo p to a coercion map but not reduction modulo p^{n. }
comment:15 in reply to: ↑ 14 Changed 4 years ago by
Replying to caruso:
IMO, It's a kinda weird to promote reduction modulo p to a coercion map but not reduction modulo p^{n. }
I agree that it's kind of weird. I'd really like for them both to be coercions, but I explain above why that doesn't seem to be possible.
I think it's probably okay to also delete the coercion to the residue field. For integers, you really want to be able to ask for a+1
if a
is an element of Zmod(N)
, but I think that this is a lot less important for padics.
If nobody else has input, I can go ahead and make this change.
comment:16 in reply to: ↑ 14 Changed 4 years ago by
Replying to caruso:
IMO, It's a kinda weird to promote reduction modulo p to a coercion map but not reduction modulo p^{n. }
I agree that it's kind of weird. I'd really like for them both to be coercions, but I explain above why that doesn't seem to be possible.
I think it's probably okay to also delete the coercion to the residue field. For integers, you really want to be able to ask for a+1
if a
is an element of Zmod(N)
, but I think that this is a lot less important for padics.
comment:17 Changed 4 years ago by
Can we try to live with a conversion? if that raise problems we cna try to make all maps coercions. But as David said, there is a precendent with other constructions in sagE.
comment:18 Changed 4 years ago by
 Commit changed from c08c76432d957b62d799581051cbab40c9766de2 to 19d02443906f72b3cd0f911e8df9de11f98c1628
Branch pushed to git repo; I updated commit sha1. New commits:
19d0244  Merge branch 'u/roed/residue_coercion' of git://trac.sagemath.org/sage into t/8240/residue_coercion

comment:19 Changed 4 years ago by
 Commit changed from 19d02443906f72b3cd0f911e8df9de11f98c1628 to 48d259dca39e9870be6c33fef6b6d5806ddfc3e7
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
6efed0b  Merge branch 'u/roed/polynomial_representation_of_a_padic_number' of git://trac.sagemath.org/sage into t/14825/polynomial_representation_of_a_padic_number

f80ca76  Working on adding iterators

030251c  Restructing of padic expansions

3c912f9  Adding documentation and making small changes to names

7e037b3  cleaning up a couple of hyperelliptic curve changes

acf6b66  Fix implementation in linear_code that used padded_list

1f8aa2e  Fix NOTES

5cf279b  Fix izip

d7d5fb6  Fixing documentation

48d259d  Merge branch 't/14825/polynomial_representation_of_a_padic_number' into t/8240/residue_coercion

comment:20 Changed 4 years ago by
 Dependencies set to #14825
comment:21 Changed 4 years ago by
 Commit changed from 48d259dca39e9870be6c33fef6b6d5806ddfc3e7 to fa7fb849cfb44527011b7bc5b9c45d3b990defa1
Branch pushed to git repo; I updated commit sha1. New commits:
fa7fb84  Remove coercions to residue field, change _element_constructor to element_class, update docstrings

comment:22 Changed 4 years ago by
 Commit changed from fa7fb849cfb44527011b7bc5b9c45d3b990defa1 to 6daea341d6b3247f5425ad453562d02c7c89826a
Branch pushed to git repo; I updated commit sha1. New commits:
6daea34  Fix doctests

comment:23 Changed 4 years ago by
 Commit changed from 6daea341d6b3247f5425ad453562d02c7c89826a to 7abedbf9507335b76e3990aedf852b27c84f3960
Branch pushed to git repo; I updated commit sha1. New commits:
7abedbf  Fix finite field coercion doctest

comment:24 Changed 4 years ago by
 Commit changed from 7abedbf9507335b76e3990aedf852b27c84f3960 to 2e38829ddb42f5d19cde07cd4aa8582d82ab03e1
Branch pushed to git repo; I updated commit sha1. New commits:
46f9cae  Merge branch 'develop' into t/14825/polynomial_representation_of_a_padic_number

e51c0f5  Merge branch 'u/roed/polynomial_representation_of_a_padic_number' of git://trac.sagemath.org/sage into t/14825/polynomial_representation_of_a_padic_number

b6457b1  Moving SEEALSO to the end of the docstring

b81b722  Remove use of depraceted list()

04a1579  Fix NOTES blocks

2e38829  Merge branch 't/14825/polynomial_representation_of_a_padic_number' into t/8240/residue_coercion

comment:25 Changed 4 years ago by
 Commit changed from 2e38829ddb42f5d19cde07cd4aa8582d82ab03e1 to 3a0ff1b42f2e0d29059af91a8043ec6e655c7f50
Branch pushed to git repo; I updated commit sha1. New commits:
3a0ff1b  Fix RST problem

comment:26 Changed 4 years ago by
I'm getting an error from the docbuild plugin:
OSError: [padics ] ...docstring of sage.rings.padics.padic_generic.pAdicGeneric.residue_ring:1: WARNING: Inline interpreted text or phrase reference startstring without endstring.
The relevant docstring is
""" Return the quotient of the ring of integers by the nth power of its maximal ideal. EXAMPLES:: sage: S.<x> = ZZ[] sage: W.<w> = Zp(5).extension(x^2  5) sage: W.residue_ring(1) Ring of integers modulo 5 The following requires implementing more general Artinian rings:: sage: W.residue_ring(2) Traceback (most recent call last): ... NotImplementedError """
Any ideas what might be going on?
comment:27 Changed 4 years ago by
in
Returns the quotient of the ring of integers by the `n`th power of the maximal ideal.
add a space after `n`
comment:28 Changed 4 years ago by
in
richcmp((type(self), self.domain(), self.codomain()), (type(other), other.domain(), other.codomain()), op)
you compare types, which is not allowed in python3.
EDIT Instead you can do something like
if type(self) != type(other): return NotImplemented return richcmp(...)
comment:29 Changed 4 years ago by
 Commit changed from 3a0ff1b42f2e0d29059af91a8043ec6e655c7f50 to a9b23a931959297810de921c1a132e6a6901d7c5
Branch pushed to git repo; I updated commit sha1. New commits:
a9b23a9  Fixing richcmp for python 3 compatibility

comment:30 Changed 4 years ago by
Thanks; I've fixed the richcmp error you pointed out.
comment:31 Changed 4 years ago by
 Commit changed from a9b23a931959297810de921c1a132e6a6901d7c5 to 9b0b60fe528ad04d136590e85211e11a75ce0c1d
Branch pushed to git repo; I updated commit sha1. New commits:
9b0b60f  Try adding a  after

comment:32 Changed 4 years ago by
 Commit changed from 9b0b60fe528ad04d136590e85211e11a75ce0c1d to d625912ad7cf2e23ac44ba2251219e87e98f0841
Branch pushed to git repo; I updated commit sha1. New commits:
d625912  Fix docstring of residue_ring

comment:33 Changed 4 years ago by
Tests pass.
comment:34 Changed 4 years ago by
I confess to not having inspected all of this code linebyline. But I did go through the main blocks, and I tried a few tests of my own without finding any issues. Positive review.
comment:35 Changed 4 years ago by
 Reviewers set to Kiran Kedlaya
 Status changed from needs_review to positive_review
comment:36 Changed 4 years ago by
 Milestone set to sage8.1
comment:38 Changed 4 years ago by
 Branch changed from u/roed/residue_coercion to u/saraedum/residue_coercion
comment:39 Changed 4 years ago by
 Commit changed from d625912ad7cf2e23ac44ba2251219e87e98f0841 to ff3d5d771e9ae100d5af264399874c2d53be5091
 Status changed from needs_work to positive_review
No conflicts for me.
New commits:
ff3d5d7  Merge remotetracking branch 'trac/develop' into t/8240/residue_coercion

comment:40 Changed 4 years ago by
 Status changed from positive_review to needs_work
wait for next beta
comment:41 Changed 4 years ago by
 Branch changed from u/saraedum/residue_coercion to u/roed/residue_coercion
comment:42 Changed 4 years ago by
 Commit changed from ff3d5d771e9ae100d5af264399874c2d53be5091 to 1fc48d1a6f4baf9625c291fdb8eae0f8d1109d3c
 Status changed from needs_work to positive_review
Fixed the merge conflict and ran all tests.
New commits:
1fc48d1  Merge commit 'd625912ad7' into t/8240/residue_coercion

comment:43 Changed 4 years ago by
 Branch changed from u/roed/residue_coercion to 1fc48d1a6f4baf9625c291fdb8eae0f8d1109d3c
 Resolution set to fixed
 Status changed from positive_review to closed
padic base coercion