Opened 4 years ago
Closed 4 years ago
#23510 closed defect (fixed)
Fraction field of fixed modulus padic rings should have floating point type
Reported by:  bsinclai  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  padics  Keywords:  sd87, beginner 
Cc:  roed  Merged in:  
Authors:  David Roe  Reviewers:  Adele Bourgeois, Julian Rüth 
Report Upstream:  N/A  Work issues:  
Branch:  189ac2b (Commits, GitHub, GitLab)  Commit:  189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59 
Dependencies:  #14825, #13591  Stopgaps: 
Description
Fraction field of fixed modulus padic rings should have floating point type.
Currently the change()
method requires that the type be specified, in this case, but instances such as the check R.fraction_field() is self
occurring in _coerce_map_from_(self, R) in padic_extension_generic.pyc.
Attachments (2)
Change History (29)
comment:1 Changed 4 years ago by
 Branch set to u/roed/fixed_mod_frac_field
comment:2 Changed 4 years ago by
 Commit set to 7926b41de6ec7eb654ae7dfcb4599b62f131f85f
 Dependencies set to #20310
 Keywords sd87 beginner added
 Status changed from new to needs_review
comment:3 Changed 4 years ago by
 Reviewers set to Adele Bourgeois
 Status changed from needs_review to positive_review
All tests pass!
comment:4 Changed 4 years ago by
 Status changed from positive_review to needs_work
sage: R.<u> = ZqFM(125, 500) sage: R.fraction_field().coerce_map_from(R) UnboundLocalError: local variable 'coerce_map' referenced before assignment
comment:5 Changed 4 years ago by
 Branch changed from u/roed/fixed_mod_frac_field to u/saraedum/fixed_mod_frac_field
comment:6 followup: ↓ 7 Changed 4 years ago by
 Commit changed from 7926b41de6ec7eb654ae7dfcb4599b62f131f85f to 16fb79bdb7f6f269726446c52a7ac6c1be18ff20
By the way, I would be in favour of implementing a padic field with fixed modulus precision (in which each padic number is truncated at the same precision encapsulated in the parent). I think it makes perfectly sense. Do you have any objections?
New commits:
16fb79b  Check that fraction_field() works as expected

comment:7 in reply to: ↑ 6 ; followup: ↓ 8 Changed 4 years ago by
Replying to caruso:
By the way, I would be in favour of implementing a padic field with fixed modulus precision (in which each padic number is truncated at the same precision encapsulated in the parent). I think it makes perfectly sense. Do you have any objections?
So, the amount of information stored in an element is unbounded. This seems problematic since computations can blow up. Why do you want to have this precision type?
comment:8 in reply to: ↑ 7 Changed 4 years ago by
Replying to roed:
So, the amount of information stored in an element is unbounded. This seems problematic since computations can blow up.
Indeed. But is it really an issue?
I noticed that similarly QpCA
does not exist and that the fraction field of ZpCA
is QpCR
and I assume that this is for the same reason, isn't it?
Why do you want to have this precision type?
I will maybe need it for implementing precision lattices for which I imagine an absolute cap (having a relative cap is certainly also interesting but does not solve the issue of unbounded elements in the framework of lattice precision since the precision may be spread out over several elements which have quite different valuations).
However, besides this, I think that QpCA
and QpFM
make sense perfectly and should be the integer ring of ZpCA
and ZpFM
respectively. I don't think that it is a good idea to switch automatically between absolute and relative precision, it should be the choice of the user.
For instance, for now, we have the following behaviour
sage: R = ZpCA(5) sage: R.fraction_field().integer_ring() 5adic Ring with capped relative precision 20 sage: R.fraction_field().integer_ring() is R False
which is, I think, a bit annoying.
comment:9 Changed 4 years ago by
 Commit changed from 16fb79bdb7f6f269726446c52a7ac6c1be18ff20 to 8814d3125ed421a7f5cbabbbfcdd3a542df13c48
Branch pushed to git repo; I updated commit sha1. New commits:
8814d31  Merge branch 'develop' into t/23510/fixed_mod_frac_field

comment:10 Changed 4 years ago by
 Branch changed from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field
comment:11 Changed 4 years ago by
 Commit changed from 8814d3125ed421a7f5cbabbbfcdd3a542df13c48 to 7e62f70fbd338a99ae80921cc585261f406bc3e7
 Dependencies changed from #20310 to #14825
 Status changed from needs_work to needs_review
Depend on #14825 to include fixes for sections that are internal to coercion system. Tests pass.
As for the discussion about adding QpFM
and QpCA
, I don't have a strong objection, but it will be very difficult to implement with the current framework in Sage. In particular, it would require two new templates, since the CR_template
and FP_template
don't have the correct precision behavior, while the CA_template
and FM_template
don't have the right data representation (they can't store elements with negative valuation). I don't think this is going to happen anytime soon.
Last 10 new commits:
7f87069  Fix segfault

b9c2fe4  Fix pickling of sections for padic coercions

6ba62dd  Fix SEEALSO again

e9c4c39  Merge branch 'u/roed/allow_exact_defining_polynomials_for_p_adic_extensions' of git://trac.sagemath.org/sage into t/23331/allow_exact_defining_polynomials_for_p_adic_extensions

561f5ac  Fix doctest errors

3142701  Merge branch 'u/roed/change_precision' of git://trac.sagemath.org/sage into t/20310/change_precision

138d939  Fix string representation doctest from #22103

1eeb367  Merge branch 't/20310/change_precision' into t/14825/polynomial_representation_of_a_padic_number

bc95b69  Merge branch 't/14825/polynomial_representation_of_a_padic_number' into t/23510/fixed_mod_frac_field

7e62f70  Fixing errors in the coercion to the fraction field for fixedmod padics

comment:12 Changed 4 years ago by
 Commit changed from 7e62f70fbd338a99ae80921cc585261f406bc3e7 to b9c3a23bdf45faf31970a107d4a45d806e8f23f8
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
2d87353  Merge branch 'develop' into t/13591/ticket/13591

9cd2d5d  Fixup the preceding merge.

5d7dbb9  Merge branch 'develop' into t/13591/ticket/13591

58c7ae6  Merge branch 'u/saraedum/ticket/13591' of git://trac.sagemath.org/sage into t/13591/add_bigoh

782df2e  Fix prec_cap > ram_prec_cap

1a4333a  Merge branch 't/13591/add_bigoh' into t/23510/fixed_mod_frac_field

538b48c  Fixing problems revealed by doctests

cdeb376  Merge branch 't/13591/add_bigoh' into t/23510/fixed_mod_frac_field

cb882fd  Update add_bigoh to account for fixedmod having a fraction field

b9c3a23  Fix _tester_add_bigoh

comment:13 Changed 4 years ago by
 Commit changed from b9c3a23bdf45faf31970a107d4a45d806e8f23f8 to a82697895556d4acda681ea0ca9f8fac8f0c51d1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a826978  Fix _tester_add_bigoh

comment:14 Changed 4 years ago by
 Dependencies changed from #14825 to #14825, #13591
comment:15 Changed 4 years ago by
 Commit changed from a82697895556d4acda681ea0ca9f8fac8f0c51d1 to 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b
Branch pushed to git repo; I updated commit sha1. New commits:
883e6b5  Fix doctest in local_generic_element.pyx

comment:16 Changed 4 years ago by
 Commit changed from 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b to 52cbd2a0ed1c9154fa25ef340274ed985da84543
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
1bb288f  protect LaTeX commands

054e5d6  clarify where _list_zero comes from

0b7fd02  default docstring layout

def3897  replace p with pi and clarify meaning of expansion

99c40d6  add unit test for expansion

6732d38  coefficients might be lists in the maximal unramified subextension

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

40737f6  Fix some of the errors in _test_expansion

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

52cbd2a  Merge branch 'u/roed/fixed_mod_frac_field' of git://trac.sagemath.org/sage into t/23510/fixed_mod_frac_field

comment:17 Changed 4 years ago by
 Branch changed from u/roed/fixed_mod_frac_field to u/saraedum/fixed_mod_frac_field
comment:18 Changed 4 years ago by
 Commit changed from 52cbd2a0ed1c9154fa25ef340274ed985da84543 to 329225966f26609551ffcf07374409c9f3149ce2
comment:19 Changed 4 years ago by
 Reviewers changed from Adele Bourgeois to Adele Bourgeois, Julian Rüth
 Status changed from needs_review to positive_review
comment:21 Changed 4 years ago by
 Branch changed from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field
comment:22 Changed 4 years ago by
 Commit changed from 329225966f26609551ffcf07374409c9f3149ce2 to 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
 Status changed from needs_work to needs_review
Thanks. Setting back to needs review for tests...
New commits:
b81b722  Remove use of depraceted list()

04a1579  Fix NOTES blocks

6764ea2  Merge branch 'develop' into t/14825/polynomial_representation_of_a_padic_number

dad02c7  Merge branch 'u/saraedum/fixed_mod_frac_field' of git://trac.sagemath.org/sage into t/23510/fixed_mod_frac_field

189ac2b  Add _test_fraction_field to the coercion tutorial

comment:23 Changed 4 years ago by
The test failures don't seem to be related to this ticket....
comment:24 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:25 Changed 4 years ago by
 Status changed from positive_review to needs_work
This does not fix the following problem:
sage: R=ZpFM(3) sage: K=R.fraction_field() sage: K.coerce_map_from(R).is_injective() NotImplementedError
comment:26 Changed 4 years ago by
 Status changed from needs_work to positive_review
Let's make this a new issue, #23965.
comment:27 Changed 4 years ago by
 Branch changed from u/roed/fixed_mod_frac_field to 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
Changing modulus and defining_polynomial to use an exact keyword
Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
minor docstring changes
Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
minor docstring fixes
add exact keyword
Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
Fix SEEALSO:
Fix exact=True errors in the right branch
Moving code for fraction_field and integer_ring, and enabling fraction fields for fixedmod