#23510 closed defect (fixed)

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

Reported by: bsinclai Owned by:
Priority: major Milestone: sage-8.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) Commit: 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
Dependencies: #14825, #13591 Stopgaps:

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 roed 21 months ago.
Diff against #14825 for ease of review
23510_over_148825_and_13591.diff (33.6 KB) - added by roed 21 months ago.
Diff against #14825 and #13591 for ease of review

Download all attachments as: .zip

Change History (29)

comment:1 Changed 21 months ago by roed

  • Branch set to u/roed/fixed_mod_frac_field

comment:2 Changed 21 months ago by roed

  • Authors set to David Roe
  • Commit set to 7926b41de6ec7eb654ae7dfcb4599b62f131f85f
  • Dependencies set to #20310
  • Keywords sd87 beginner added
  • Status changed from new to needs_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 21 months ago by abourgeois

  • Reviewers set to Adele Bourgeois
  • Status changed from needs_review to positive_review

All tests pass!

comment:4 Changed 21 months ago by saraedum

  • 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 21 months ago by saraedum

  • Branch changed from u/roed/fixed_mod_frac_field to u/saraedum/fixed_mod_frac_field

comment:6 follow-up: Changed 21 months ago by caruso

  • Commit changed from 7926b41de6ec7eb654ae7dfcb4599b62f131f85f to 16fb79bdb7f6f269726446c52a7ac6c1be18ff20

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 ; follow-up: Changed 21 months ago by roed

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 21 months ago by 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 21 months ago by git

  • Commit changed from 16fb79bdb7f6f269726446c52a7ac6c1be18ff20 to 8814d3125ed421a7f5cbabbbfcdd3a542df13c48

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

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

comment:10 Changed 21 months ago by roed

  • Branch changed from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field

comment:11 Changed 21 months ago by roed

  • 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:

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 21 months ago by roed

Diff against #14825 for ease of review

comment:12 Changed 21 months ago by git

  • Commit changed from 7e62f70fbd338a99ae80921cc585261f406bc3e7 to b9c3a23bdf45faf31970a107d4a45d806e8f23f8

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 21 months ago by git

  • Commit changed from b9c3a23bdf45faf31970a107d4a45d806e8f23f8 to a82697895556d4acda681ea0ca9f8fac8f0c51d1

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

a826978Fix _tester_add_bigoh

comment:14 Changed 21 months ago by roed

  • Dependencies changed from #14825 to #14825, #13591

Changed 21 months ago by roed

Diff against #14825 and #13591 for ease of review

comment:15 Changed 21 months ago by git

  • Commit changed from a82697895556d4acda681ea0ca9f8fac8f0c51d1 to 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b

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

883e6b5Fix doctest in local_generic_element.pyx

comment:16 Changed 20 months ago by git

  • Commit changed from 883e6b5fe81e5c2741ef8fb3521039ac6470fd0b to 52cbd2a0ed1c9154fa25ef340274ed985da84543

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 19 months ago by saraedum

  • Branch changed from u/roed/fixed_mod_frac_field to u/saraedum/fixed_mod_frac_field

comment:18 Changed 19 months ago by git

  • Commit changed from 52cbd2a0ed1c9154fa25ef340274ed985da84543 to 329225966f26609551ffcf07374409c9f3149ce2

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 19 months ago by saraedum

  • Reviewers changed from Adele Bourgeois to Adele Bourgeois, Julian Rüth
  • Status changed from needs_review to positive_review

comment:20 Changed 19 months ago by vbraun

  • Status changed from positive_review to needs_work

See patchbot

comment:21 Changed 19 months ago by roed

  • Branch changed from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field

comment:22 Changed 19 months ago by roed

  • Commit changed from 329225966f26609551ffcf07374409c9f3149ce2 to 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
  • Status changed from needs_work to needs_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 19 months ago by roed

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

comment:24 Changed 19 months ago by saraedum

  • Status changed from needs_review to positive_review

comment:25 Changed 19 months ago by saraedum

  • 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 19 months ago by saraedum

  • Status changed from needs_work to positive_review

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

comment:27 Changed 19 months ago by vbraun

  • Branch changed from u/roed/fixed_mod_frac_field to 189ac2b4a6b37dbe49ea70ab09e6e68b8b091d59
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.