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

Priority:  major  Milestone:  sage8.1 
Component:  padics  Keywords:  sd87, beginner 
Cc:  David Roe  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 5 years ago by
Branch:  → u/roed/fixed_mod_frac_field 

comment:2 Changed 5 years ago by
Authors:  → David Roe 

Commit:  → 7926b41de6ec7eb654ae7dfcb4599b62f131f85f 
Dependencies:  → #20310 
Keywords:  sd87 beginner added 
Status:  new → needs_review 
comment:3 Changed 5 years ago by
Reviewers:  → Adele Bourgeois 

Status:  needs_review → positive_review 
All tests pass!
comment:4 Changed 5 years ago by
Status:  positive_review → 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 5 years ago by
Branch:  u/roed/fixed_mod_frac_field → u/saraedum/fixed_mod_frac_field 

comment:6 followup: 7 Changed 5 years ago by
Commit:  7926b41de6ec7eb654ae7dfcb4599b62f131f85f → 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 followup: 8 Changed 5 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 Changed 5 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 5 years ago by
Commit:  16fb79bdb7f6f269726446c52a7ac6c1be18ff20 → 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 5 years ago by
Branch:  u/saraedum/fixed_mod_frac_field → u/roed/fixed_mod_frac_field 

comment:11 Changed 5 years ago by
Commit:  8814d3125ed421a7f5cbabbbfcdd3a542df13c48 → 7e62f70fbd338a99ae80921cc585261f406bc3e7 

Dependencies:  #20310 → #14825 
Status:  needs_work → 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

Changed 5 years ago by
Attachment:  23510_over_14825.diff added 

Diff against #14825 for ease of review
comment:12 Changed 5 years ago by
Commit:  7e62f70fbd338a99ae80921cc585261f406bc3e7 → 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 5 years ago by
Commit:  b9c3a23bdf45faf31970a107d4a45d806e8f23f8 → 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 5 years ago by
Dependencies:  #14825 → #14825, #13591 

Changed 5 years ago by
Attachment:  23510_over_148825_and_13591.diff added 

comment:15 Changed 5 years ago by
Commit:  a82697895556d4acda681ea0ca9f8fac8f0c51d1 → 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b 

Branch pushed to git repo; I updated commit sha1. New commits:
883e6b5  Fix doctest in local_generic_element.pyx

comment:16 Changed 5 years ago by
Commit:  883e6b5fe81e5c2741ef8fb3521039ac6470fd0b → 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 5 years ago by
Branch:  u/roed/fixed_mod_frac_field → u/saraedum/fixed_mod_frac_field 

comment:18 Changed 5 years ago by
Commit:  52cbd2a0ed1c9154fa25ef340274ed985da84543 → 329225966f26609551ffcf07374409c9f3149ce2 

comment:19 Changed 5 years ago by
Reviewers:  Adele Bourgeois → Adele Bourgeois, Julian Rüth 

Status:  needs_review → positive_review 
comment:21 Changed 5 years ago by
Branch:  u/saraedum/fixed_mod_frac_field → u/roed/fixed_mod_frac_field 

comment:22 Changed 5 years ago by
Commit:  329225966f26609551ffcf07374409c9f3149ce2 → 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59 

Status:  needs_work → 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:24 Changed 5 years ago by
Status:  needs_review → positive_review 

comment:25 Changed 5 years ago by
Status:  positive_review → 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 5 years ago by
Status:  needs_work → positive_review 

Let's make this a new issue, #23965.
comment:27 Changed 5 years ago by
Branch:  u/roed/fixed_mod_frac_field → 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59 

Resolution:  → fixed 
Status:  positive_review → 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