Opened 5 years ago

Closed 5 years ago

#23331 closed enhancement (fixed)

Allow exact defining polynomials for p-adic extensions

Reported by: David Roe Owned by:
Priority: major Milestone: sage-8.0
Component: padics Keywords: sd87
Cc: Julian Rüth Merged in:
Authors: David Roe Reviewers: Julian Rüth
Report Upstream: N/A Work issues:
Branch: 561f5ac (Commits, GitHub, GitLab) Commit: 561f5ac333b00f5e4a2ee70b82e2b5e4965876b3
Dependencies: #20073 Stopgaps:

Status badges

Description (last modified by Julian Rüth)

Currently, the defining polynomial of a p-adic extension always has coefficients over the base ring:

sage: R.<x> = ZZ[]
sage: W.<w> = Zp(5).extension(x^2 - 5)
sage: W.defining_polynomial().base_ring()
5-adic Ring with capped relative precision 20

This is fine, one would expect the defining polynomial to have coefficients in the base ring but we would also like to have access to the exact polynomial. To make this consistent, we should always keep two polynomials: a defining polynomial and an exact polynomial.

The defining polynomial has coefficients over the base ring. It is not part of the key used in the factory as it can be recovered from the exact polynomial.

The exact polynomial has coefficients in a number field: For extension of Zp/Qp, the coefficients are in QQ. For two step extensions, the coefficients are in the absolute number field defined by the exact polynomial of the base ring. For more general extensions, the coefficients are in the tower of number fields, defined by the exact polynomials.

The exact polynomial is part of the key used in the factory. Two fields with different exact polynomial but same defining polynomial are different. This makes sense because they behave differently with respect to change() when changing the precision.

For extensions constructed from a modulus with inexact coefficients (in the base ring) we just set the exact polynomial to an approximation of the modulus.

Change History (25)

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

Description: modified (diff)

comment:2 Changed 5 years ago by David Roe

Branch: u/roed/allow_exact_defining_polynomials_for_p_adic_extensions

comment:3 Changed 5 years ago by David Roe

Commit: dbbc631feff9dbf4f3a37825329263d640290202
Status: newneeds_review

I ended up doing some additional things:

  • Remove some old code related to lazy p-adics (since I was changing the key format anyway)
  • Change the printing of unramified and eisenstein extensions

New commits:

7e21e77Adding an exact modulus to p-adic rings and fields, changing printing for p-adic extensions, removing some vestigal lazy p-adic code
dbbc631Fix pickles

comment:4 Changed 5 years ago by git

Commit: dbbc631feff9dbf4f3a37825329263d6402902026cb9d22e018af5672f54d2ba657ae354283098a8

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

6cb9d22Remove extraneous newlines in padic_ZZ_pX_FM_element.pyx

comment:5 Changed 5 years ago by Xavier Caruso

Awesome! Some comments through:

  • Why did you implement only exact_field and not exact_ring (for p-adic rings)?
  • Shouldn't we have a coercion map from the exact subfield/subring to the p-adic field/ring?
  • I would say that the methods modulus and defining_polynomial should both by default return a polynomial defined over the p-adic field and accept an attribute specifying whether the user wants an exact defining polynomial (I believe that having different behaviour for these two methods is quite confusing).
  • Shouldn't we check that the precision of the defining polynomial is enough to well-defined extension (thanks to Krasner's Lemma)?

comment:6 Changed 5 years ago by David Roe

Dependencies: #20073

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

Status: needs_reviewneeds_work

Replying to caruso:

Awesome! Some comments through:

  • Why did you implement only exact_field and not exact_ring (for p-adic rings)?

Two reasons:

  1. We didn't need it for this ticket
  2. Eventually, exact rings would require computing rings of integers in number fields, or at least p-maximal orders. We don't want to solve a hard computational problem problem to do something trivial.
  • Shouldn't we have a coercion map from the exact subfield/subring to the p-adic field/ring?

Yes. I'll add this to the the followup ticket #23471

  • I would say that the methods modulus and defining_polynomial should both by default return a polynomial defined over the p-adic field and accept an attribute specifying whether the user wants an exact defining polynomial (I believe that having different behaviour for these two methods is quite confusing).

I had a conversation with Julian, Marc and Rob and we agree.

  • Shouldn't we check that the precision of the defining polynomial is enough to well-defined extension (thanks to Krasner's Lemma)?

Yes, but I think that this should be saved for a followup ticket #23480 (Julian has code for doing this, but it depends on the MacLane? package).

comment:8 Changed 5 years ago by git

Commit: 6cb9d22e018af5672f54d2ba657ae354283098a8921af5e95be38203770443d6d9f6e8c9f123d269

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

7f96697Fixed typo with variable assignment
bbb7268added doctests
ee461b6merge with develop
8081eabadded doctests after fixing conflicts
063b58cMerge branch 'develop' into t/20073/ticket/20073
48be601Added documentation to verify that the extension has the right defining polynomial
921af5eChanging modulus and defining_polynomial to use an exact keyword

comment:9 Changed 5 years ago by David Roe

Status: needs_workneeds_review

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

Authors: David Roe
Reviewers: Julian Rüth

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

Branch: u/roed/allow_exact_defining_polynomials_for_p_adic_extensionsu/saraedum/allow_exact_defining_polynomials_for_p_adic_extensions

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

Commit: 921af5e95be38203770443d6d9f6e8c9f123d26977779eabdebc98cb503fe2e432a90745c8390aab

Positive review once the patchbot is happy.


New commits:

77779eaminor docstring changes

comment:13 Changed 5 years ago by David Roe

Branch: u/saraedum/allow_exact_defining_polynomials_for_p_adic_extensionsu/roed/allow_exact_defining_polynomials_for_p_adic_extensions

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

Branch: u/roed/allow_exact_defining_polynomials_for_p_adic_extensionsu/saraedum/allow_exact_defining_polynomials_for_p_adic_extensions

comment:15 Changed 5 years ago by David Roe

Branch: u/saraedum/allow_exact_defining_polynomials_for_p_adic_extensionsu/roed/allow_exact_defining_polynomials_for_p_adic_extensions

comment:16 Changed 5 years ago by git

Commit: 77779eabdebc98cb503fe2e432a90745c8390aab6ba62dd02f9c85ee49ca06a68d99cf724c264b96

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

6ba62ddFix SEEALSO again

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

Status: needs_reviewpositive_review

comment:18 in reply to:  7 ; Changed 5 years ago by Xavier Caruso

Replying to roed:

  1. Eventually, exact rings would require computing rings of integers in number fields, or at least p-maximal orders. We don't want to solve a hard computational problem problem to do something trivial.

I don't unserstand why implementing exact_ring would require something more than just calling ZZ.extension(...) that creates the order in a number field defined by the given polynomial. We do not care if it is not the maximal order, don't we?

Shouldn't we have a coercion map from the exact subfield/subring to the p-adic field/ring?

Yes. I'll add this to the the followup ticket #23471

Is it in it? (I ask the question because this ticket is not marked as a dependency of #23471.)

comment:19 in reply to:  18 Changed 5 years ago by David Roe

Replying to caruso:

Replying to roed:

  1. Eventually, exact rings would require computing rings of integers in number fields, or at least p-maximal orders. We don't want to solve a hard computational problem problem to do something trivial.

I don't unserstand why implementing exact_ring would require something more than just calling ZZ.extension(...) that creates the order in a number field defined by the given polynomial. We do not care if it is not the maximal order, don't we?

Good point. See #23507.

Shouldn't we have a coercion map from the exact subfield/subring to the p-adic field/ring?

Yes. I'll add this to the the followup ticket #23471

Is it in it? (I ask the question because this ticket is not marked as a dependency of #23471.)

This ticket is an indirect dependency, through #20310.

Last edited 5 years ago by David Roe (previous) (diff)

comment:20 Changed 5 years ago by Volker Braun

Status: positive_reviewneeds_work

Merge conflict, wait for next beta...

comment:21 Changed 5 years ago by git

Commit: 6ba62dd02f9c85ee49ca06a68d99cf724c264b96e9c4c394066d9ce451b500a2ddf9572a488d006f

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

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

comment:22 Changed 5 years ago by git

Commit: e9c4c394066d9ce451b500a2ddf9572a488d006f561f5ac333b00f5e4a2ee70b82e2b5e4965876b3

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

561f5acFix doctest errors

comment:23 Changed 5 years ago by David Roe

Status: needs_workpositive_review

I fixed the merge conflict and ran all tests (fixing two).

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

There are patchbot failures. But these seem to be in develop already.

comment:25 Changed 5 years ago by Volker Braun

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