Opened 5 years ago

Closed 5 years ago

#23510 closed defect (fixed)

Fraction field of fixed modulus p-adic rings should have floating point type

Reported by: Brian Sinclair Owned by:
Priority: major Milestone: sage-8.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:

Status badges

Description

Fraction field of fixed modulus p-adic 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)

23510_over_14825.diff (31.5 KB) - added by David Roe 5 years ago.
Diff against #14825 for ease of review
23510_over_148825_and_13591.diff (33.6 KB) - added by David Roe 5 years ago.
Diff against #14825 and #13591 for ease of review

Download all attachments as: .zip

Change History (29)

comment:1 Changed 5 years ago by David Roe

Branch: u/roed/fixed_mod_frac_field

comment:2 Changed 5 years ago by David Roe

Authors: David Roe
Commit: 7926b41de6ec7eb654ae7dfcb4599b62f131f85f
Dependencies: #20310
Keywords: sd87 beginner added
Status: newneeds_review

Last 10 new commits:

921af5eChanging modulus and defining_polynomial to use an exact keyword
39043f1Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
77779eaminor docstring changes
6e2495fMerge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
7428353minor docstring fixes
bd15d71add exact keyword
99dccf6Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
ef4ed33Fix SEEALSO:
1754b44Fix exact=True errors in the right branch
7926b41Moving code for fraction_field and integer_ring, and enabling fraction fields for fixed-mod

comment:3 Changed 5 years ago by Adèle Bourgeois

Reviewers: Adele Bourgeois
Status: needs_reviewpositive_review

All tests pass!

comment:4 Changed 5 years ago by Julian Rüth

Status: positive_reviewneeds_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 Julian Rüth

Branch: u/roed/fixed_mod_frac_fieldu/saraedum/fixed_mod_frac_field

comment:6 Changed 5 years ago by Xavier Caruso

Commit: 7926b41de6ec7eb654ae7dfcb4599b62f131f85f16fb79bdb7f6f269726446c52a7ac6c1be18ff20

By the way, I would be in favour of implementing a p-adic field with fixed modulus precision (in which each p-adic number is truncated at the same precision encapsulated in the parent). I think it makes perfectly sense. Do you have any objections?


New commits:

16fb79bCheck that fraction_field() works as expected

comment:7 in reply to:  6 ; Changed 5 years ago by David Roe

Replying to caruso:

By the way, I would be in favour of implementing a p-adic field with fixed modulus precision (in which each p-adic 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 5 years ago by Xavier Caruso

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()
    5-adic 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 git

Commit: 16fb79bdb7f6f269726446c52a7ac6c1be18ff208814d3125ed421a7f5cbabbbfcdd3a542df13c48

Branch pushed to git repo; I updated commit sha1. New commits:

8814d31Merge branch 'develop' into t/23510/fixed_mod_frac_field

comment:10 Changed 5 years ago by David Roe

Branch: u/saraedum/fixed_mod_frac_fieldu/roed/fixed_mod_frac_field

comment:11 Changed 5 years ago by David Roe

Commit: 8814d3125ed421a7f5cbabbbfcdd3a542df13c487e62f70fbd338a99ae80921cc585261f406bc3e7
Dependencies: #20310#14825
Status: needs_workneeds_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:

7f87069Fix segfault
b9c2fe4Fix pickling of sections for p-adic coercions
6ba62ddFix SEEALSO again
e9c4c39Merge 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
561f5acFix doctest errors
3142701Merge branch 'u/roed/change_precision' of git://trac.sagemath.org/sage into t/20310/change_precision
138d939Fix string representation doctest from #22103
1eeb367Merge branch 't/20310/change_precision' into t/14825/polynomial_representation_of_a_padic_number
bc95b69Merge branch 't/14825/polynomial_representation_of_a_padic_number' into t/23510/fixed_mod_frac_field
7e62f70Fixing errors in the coercion to the fraction field for fixed-mod p-adics

Changed 5 years ago by David Roe

Attachment: 23510_over_14825.diff added

Diff against #14825 for ease of review

comment:12 Changed 5 years ago by git

Commit: 7e62f70fbd338a99ae80921cc585261f406bc3e7b9c3a23bdf45faf31970a107d4a45d806e8f23f8

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

2d87353Merge branch 'develop' into t/13591/ticket/13591
9cd2d5dFixup the preceding merge.
5d7dbb9Merge branch 'develop' into t/13591/ticket/13591
58c7ae6Merge branch 'u/saraedum/ticket/13591' of git://trac.sagemath.org/sage into t/13591/add_bigoh
782df2eFix prec_cap -> ram_prec_cap
1a4333aMerge branch 't/13591/add_bigoh' into t/23510/fixed_mod_frac_field
538b48cFixing problems revealed by doctests
cdeb376Merge branch 't/13591/add_bigoh' into t/23510/fixed_mod_frac_field
cb882fdUpdate add_bigoh to account for fixed-mod having a fraction field
b9c3a23Fix _tester_add_bigoh

comment:13 Changed 5 years ago by git

Commit: b9c3a23bdf45faf31970a107d4a45d806e8f23f8a82697895556d4acda681ea0ca9f8fac8f0c51d1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a826978Fix _tester_add_bigoh

comment:14 Changed 5 years ago by David Roe

Dependencies: #14825#14825, #13591

Changed 5 years ago by David Roe

Diff against #14825 and #13591 for ease of review

comment:15 Changed 5 years ago by git

Commit: a82697895556d4acda681ea0ca9f8fac8f0c51d1883e6b5fe81e5c2741ef8fb3521039ac6470fd0b

Branch pushed to git repo; I updated commit sha1. New commits:

883e6b5Fix doctest in local_generic_element.pyx

comment:16 Changed 5 years ago by git

Commit: 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b52cbd2a0ed1c9154fa25ef340274ed985da84543

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

1bb288fprotect LaTeX commands
054e5d6clarify where _list_zero comes from
0b7fd02default docstring layout
def3897replace p with pi and clarify meaning of expansion
99c40d6add unit test for expansion
6732d38coefficients might be lists in the maximal unramified subextension
acc606aMerge branch 'u/saraedum/polynomial_representation_of_a_padic_number' of git://trac.sagemath.org/sage into t/14825/polynomial_representation_of_a_padic_number
40737f6Fix some of the errors in _test_expansion
6efed0bMerge branch 'u/roed/polynomial_representation_of_a_padic_number' of git://trac.sagemath.org/sage into t/14825/polynomial_representation_of_a_padic_number
52cbd2aMerge 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 Julian Rüth

Branch: u/roed/fixed_mod_frac_fieldu/saraedum/fixed_mod_frac_field

comment:18 Changed 5 years ago by git

Commit: 52cbd2a0ed1c9154fa25ef340274ed985da84543329225966f26609551ffcf07374409c9f3149ce2

Branch pushed to git repo; I updated commit sha1. New commits:

fee575dMerge branch 't/14825/polynomial_representation_of_a_padic_number' into t/23510/fixed_mod_frac_field
3292259Merge branch 'u/saraedum/fixed_mod_frac_field' of git://trac.sagemath.org/sage into t/23510/fixed_mod_frac_field

comment:19 Changed 5 years ago by Julian Rüth

Reviewers: Adele BourgeoisAdele Bourgeois, Julian Rüth
Status: needs_reviewpositive_review

comment:20 Changed 5 years ago by Volker Braun

Status: positive_reviewneeds_work

See patchbot

comment:21 Changed 5 years ago by David Roe

Branch: u/saraedum/fixed_mod_frac_fieldu/roed/fixed_mod_frac_field

comment:22 Changed 5 years ago by David Roe

Commit: 329225966f26609551ffcf07374409c9f3149ce2189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
Status: needs_workneeds_review

Thanks. Setting back to needs review for tests...


New commits:

b81b722Remove use of depraceted list()
04a1579Fix NOTES blocks
6764ea2Merge branch 'develop' into t/14825/polynomial_representation_of_a_padic_number
dad02c7Merge branch 'u/saraedum/fixed_mod_frac_field' of git://trac.sagemath.org/sage into t/23510/fixed_mod_frac_field
189ac2bAdd _test_fraction_field to the coercion tutorial

comment:23 Changed 5 years ago by David Roe

The test failures don't seem to be related to this ticket....

comment:24 Changed 5 years ago by Julian Rüth

Status: needs_reviewpositive_review

comment:25 Changed 5 years ago by Julian Rüth

Status: positive_reviewneeds_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 Julian Rüth

Status: needs_workpositive_review

Let's make this a new issue, #23965.

comment:27 Changed 5 years ago by Volker Braun

Branch: u/roed/fixed_mod_frac_field189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.