Reimplement elements of Zp and Qp using templates

[roed314]

The goal of this ticket is to reimplement elements of Zp and Qp using the ideas from sage/rings/polynomial/polynomial_template.pxi. This should make p-adics in Sage both easier to maintain and easier to add new classes.

The final patch should not affect the external interface to Zp and Qp.

---

Apply

• attachment: trac_12555.patch
• attachment: trac_12555_review.patch
• attachment: 12555_rebase_6223.patch
• attachment: trac_12555_doc_improvements.patch
• attachment: trac_12555-pdf_fix-ts.patch

Depends on #12625
Depends on #13299
Depends on #14287
Depends on #12575
Depends on #6223

CC: @sagetrac-sydahmad @RalphieBoy @williamstein @sagetrac-JStarx @jpflori

Component: padics

Keywords: sd53 padics templates

Author: David Roe, Julian Rueth

Branch/Commit: public/padics/templates-12555 @ aa65f59

Reviewer: William Stein, Soroosh Yazdani, Julian Rueth, Travis Scrimshaw

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

[sagetrac-sydahmad]

Replying to @roed314:

The goal of this ticket is to reimplement elements of Zp and Qp using the ideas from sage/rings/polynomial/polynomial_template.pxi. This should make p-adics in Sage both easier to maintain and easier to add new classes.

[roed314]

comment:3

Now includes capped absolute and fixed modulus elements. There are a few problems remaining:

• teichmuller_list is disabled since it had a bug and I didn't want to fix it at the time. I'll track this down later.
• Some coercions aren't working since the element class has changed.

This version should be enough for Justin to compare the performance of the templated and non-templated versions. I'm now going to create another patch on top of this which removes the untemplated versions.

[roed314]

Attachment: 12555.patch.gz

Still some bugs, but it should be useable to compare timings

[roed314]

I've fixed all known bugs; currently doctesting

[roed314]

Attachment: 12555_2.patch.gz

Attachment: 12555_printing.patch.gz

[roed314]

Attachment: 12555_remove_base_coerce.patch.gz

[roed314]

Attachment: 12555_conv.patch.gz

Attachment: 12555_remove_base.patch.gz

[roed314]

Attachment: 12555_linkage.patch.gz

[roed314]

Attachment: 12555_template.patch.gz

[roed314]

Attachment: 12555_FM.patch.gz

Attachment: 12555_CR.patch.gz

[roed314]

Attachment: 12555_CA.patch.gz

[roed314]

Author: David Roe

[roed314]

Dependencies: #12625, #12575

[williamstein]

this is my REFEREE patch after reading through half the code. I stopped at padic_ZZ_pX_FM_element.pyx. This is the ultimate patch bomb. Very hard to referee...

[saraedum]

comment:6

Attachment: trac_12555-referee.patch.gz

A review of this patch is at https://github.com/saraedum/padic-review/commit. We agreed to work on the remaining issues there.

[saraedum]

Reviewer: William Stein, Soroosh Yazdani, Julian Rueth

[roed314]

Changed dependencies from #12625, #12575 to #12625, #12575, #14106

[roed314]

comment:8

I've made a pull request to https://github.com/saraedum/padic-review/commit, addressing the comments. Tests all pass for me.

We had discussed having a document that runs some overall tests. I haven't made that yet, but can this week. If someone has time to take a look at this this week, I'll be available to make changes....

[saraedum]

comment:9

Replying to @roed314:

I've made a pull request to https://github.com/saraedum/padic-review/commit, addressing the comments. Tests all pass for me.

Great.

We had discussed having a document that runs some overall tests. I haven't made that yet, but can this week. If someone has time to take a look at this this week, I'll be available to make changes....

You haven't started to work on this already, have you? I just started to work on these tests. I'll push to github as soon as I have something working.

[sagetrac-git]

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

fcf6ad2

Fix for comparison of padics.

[sagetrac-git]

Changed commit from 5f00813 to fcf6ad2

[jpflori]

comment:78

Should be ok now.
May be worth putting an additional test in there, but I'm too tired now and the code is already tested by the previous bug.

[tscrim]

comment:79

That example (and testing the files) now works for me, so it's back to positive review.

EDIT - also I don't think .pxi files are run as part of doctesting?

[vbraun]

comment:81

Documentation doesn't build

[padics   ] /mnt/SSD1/mod_buildslave/sage_git/build/src/doc/en/reference/padics/sage/rings/padics/padic_base_generic_element.rst:11: WARNING: autodoc can't import/find module 'sage.rings.padics.padic_base_generic_element', it reported error: "No module named padic_base_generic_element", please check your spelling and sys.path
[padics   ] docstring of sage.rings.padics.padic_fixed_mod_element:9: ERROR: Unknown directive type "NOTES".

[sagetrac-git]

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

f338b7f

Fix wrong NOTE block.

[sagetrac-git]

Changed commit from fcf6ad2 to f338b7f

[sagetrac-git]

Changed commit from f338b7f to aa65f59

[sagetrac-git]

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

aa65f59

Removed removed file from doc.

[tscrim]

New commits:

aa65f59

Removed removed file from doc.

[nilesjohnson]

comment:86

There are a number of other power series bugs that have been waiting in one way or another for rewriting of padics. Could someone here take a look and comment on the appropriate tickets? They do not seem to be resolved by this one.

#9457: Buggy equality of power series -- The fix seems straightforward, but causes a bizarre doctest failure in sha_tate.py

#4656: Power series over p-adics treat inexact zeros inconsistently -- The fix here causes the same doctest to fail.

#8198: p-adic precision is dropped when matrix acts on vector -- Possibly a root cause of the previous two bugs.

#5075: Polynomials over inexact rings truncate inexact leading zeroes -- This was somehow fixed for polynomials over p-adics, but polynomials over other power series still have the issue. David Roe commented that it may be fixed as part of a larger patch (I hope there's not another one after this!)

[jdemeyer]

comment:88

See #15653 for a blocker follow-up.