Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sagetrac-bsinclai mannequin opened this issue Jul 21, 2017 · 43 comments
Closed

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

sagetrac-bsinclai mannequin opened this issue Jul 21, 2017 · 43 comments

Comments

@sagetrac-bsinclai
Copy link
Mannequin

sagetrac-bsinclai mannequin commented Jul 21, 2017

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.

Depends on #14825
Depends on #13591

CC: @roed314

Component: padics

Keywords: sd87

Author: David Roe

Branch/Commit: 189ac2b

Reviewer: Adele Bourgeois, Julian Rüth

Issue created by migration from https://trac.sagemath.org/ticket/23510

@sagetrac-bsinclai sagetrac-bsinclai mannequin added this to the sage-8.1 milestone Jul 21, 2017
@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

Branch: u/roed/fixed_mod_frac_field

@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

Dependencies: #20310

@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

Author: David Roe

@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

Commit: 7926b41

@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

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

@roed314
Copy link
Contributor

roed314 commented Jul 21, 2017

Changed keywords from none to sd87

@sagetrac-abourgeois
Copy link
Mannequin

sagetrac-abourgeois mannequin commented Jul 21, 2017

Reviewer: Adele Bourgeois

@sagetrac-abourgeois
Copy link
Mannequin

sagetrac-abourgeois mannequin commented Jul 21, 2017

comment:3

All tests pass!

@saraedum
Copy link
Member

comment:4
sage: R.<u> = ZqFM(125, 500)
sage: R.fraction_field().coerce_map_from(R)
UnboundLocalError: local variable 'coerce_map' referenced before assignment

@saraedum
Copy link
Member

Changed branch from u/roed/fixed_mod_frac_field to u/saraedum/fixed_mod_frac_field

@xcaruso
Copy link
Contributor

xcaruso commented Jul 29, 2017

Changed commit from 7926b41 to 16fb79b

@xcaruso
Copy link
Contributor

xcaruso commented Jul 29, 2017

comment:6

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

@roed314
Copy link
Contributor

roed314 commented Jul 29, 2017

comment:7

Replying to @xcaruso:

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?

@xcaruso
Copy link
Contributor

xcaruso commented Jul 30, 2017

comment:8

Replying to @roed314:

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2017

Changed commit from 16fb79b to 8814d31

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2017

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

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

@roed314
Copy link
Contributor

roed314 commented Aug 3, 2017

Changed branch from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field

@roed314
Copy link
Contributor

roed314 commented Aug 3, 2017

Changed commit from 8814d31 to 7e62f70

@roed314
Copy link
Contributor

roed314 commented Aug 3, 2017

comment:11

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

@roed314
Copy link
Contributor

roed314 commented Aug 3, 2017

Changed dependencies from #20310 to #14825

@saraedum
Copy link
Member

Changed reviewer from Adele Bourgeois to Adele Bourgeois, Julian Rüth

@vbraun
Copy link
Member

vbraun commented Sep 23, 2017

comment:20

See patchbot

@roed314
Copy link
Contributor

roed314 commented Sep 23, 2017

Changed branch from u/saraedum/fixed_mod_frac_field to u/roed/fixed_mod_frac_field

@roed314
Copy link
Contributor

roed314 commented Sep 23, 2017

comment:22

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

@roed314
Copy link
Contributor

roed314 commented Sep 23, 2017

Changed commit from 3292259 to 189ac2b

@roed314
Copy link
Contributor

roed314 commented Sep 25, 2017

comment:23

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

@saraedum
Copy link
Member

saraedum commented Oct 5, 2017

comment:25

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

@saraedum
Copy link
Member

saraedum commented Oct 5, 2017

comment:26

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

@vbraun
Copy link
Member

vbraun commented Oct 5, 2017

Changed branch from u/roed/fixed_mod_frac_field to 189ac2b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants