New implementation of unramified p-adics using FLINT and templates

[roed]

Reimplement Qq using FLINT.

[roed]

This ticket is essentially complete at ​https://github.com/saraedum/sage/tree/Zq. Once #12173 is positively reviewed we'll make some patches and put this up for review.

[jdemeyer]

Replying to roed:

Once #12173 is positively reviewed

check!

[saraedum]

The repository moved to ​https://github.com/saraedum/sage-renamed/tree/Zq

[roed]

Last 10 new commits:

​ed8db42	fixup
​244b3a8	conversion from finite fields
​8f255a9	fix some typos
​65d3c03	added matrix_mod_pn
​ff52ce0	removed FLINT skip
​7b2ad9a	fixed a doctest
​fd2ea9f	added flint rep
​13a2d60	added an _new_fmpz_poly method
​52010f3	switched to powcomputer_
​02ffe57	disabled tests for FLINT

[roed]

New commits:

​e7afa8f	Improved some_elements()
​ef18f02	Adding some docstrings, fixing a few typos.

[saraedum]

I mostly looked at the last two commits. Everything else had already been reviewed before. I did not run doctests. Is there a working patchbot which will run the doctests or do I have to run them manually?

[vbraun]

Does not work... please run "make ptestlong"

[roed]

Will do.

[git]

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

​98a4529

Fix doctest errors

[jdemeyer]

There is invalid use of sig_on() in many places. The following is wrong:

sig_on()
if condition:
    raise Exception
sig_off()

You must put a sig_off() before the raise (or use try/finally).

I also recommend to doctest these exceptions. The doctesting framework catches such invalid use of sig_on().

I also don't like bare except: statements. Be explicit and use except BaseException: or except Exception: instead.

[git]

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

​d0badde

Fixing sig_on/sig_off pairs, adding BaseException to bare except:s

[roed]

Replying to jdemeyer:

There is invalid use of sig_on() in many places. The following is wrong:

sig_on()
if condition:
    raise Exception
sig_off()

You must put a sig_off() before the raise (or use try/finally).

Thanks. I fixed them.

I also recommend to doctest these exceptions. The doctesting framework catches such invalid use of sig_on().

All of the instances I found were for problems that we never expect to arise (out of memory errors, etc). So I don't see a reasonable way to doctest these exceptions.

I also don't like bare except: statements. Be explicit and use except BaseException: or except Exception: instead.

I know. :-) I told Julian you didn't like them, but he claimed it was okay since we raised after catching in each case. Anyway, I've changed them to except BaseException:.

[vbraun]

For the record, the Python docs say

exception BaseException

The base class for all built-in exceptions. It is not meant to be directly inherited by user-defined classes (for that, use Exception).

Nested exception blocks are generally hard to read, quadruply nested exception try/except/finally blocks even more so. Besides the usual advice of breaking long functions up into shorter ones, for the heap cleanup in the face of exceptions make sure that invalid / not yet allocated pointers are NULL. To simplify the cleanup part, free(NULL) is a no-op.

The for i from 0 <= i <= n construct is old, Cython nowadays produces the same C code for the (preferred) Python syntax for i in range(n).

[jdemeyer]

Replying to roed:

but he claimed it was okay since we raised after catching in each case. Anyway, I've changed them to except BaseException:.

It is technically ok in this case. The reason I don't like except: is that in almost all cases that I have seen, it was a mistake. If you write except BaseException:, it shows that you probably know what you're doing and that it is not a mistake. It is also consistent with "Explicit is better than implicit" from the Zen of Python (import this).

[jdemeyer]

sig_on() should be outside the try/finally block (if sig_on() raises a KeyboardInterrupt, then sig_off() should not be called)

sig_on()
try:
    ...
finally:
    sig_off()

[git]

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

​e101508	Some sig_on, cython for loop fixes
​c647f4a	More sig_on, cython for loop fixes

[roed]

Nested exception blocks are generally hard to read, quadruply nested exception try/except/finally blocks even more so. Besides the usual advice of breaking long functions up into shorter ones, for the heap cleanup in the face of exceptions make sure that invalid / not yet allocated pointers are NULL. To simplify the cleanup part, free(NULL) is a no-op.

What about things like mpz_clear, fmpz_clear and fmpz_poly_clear? If a is an fmpz_poly_t that has not yet had fmpz_poly_init(a) run on it, won't fmpz_poly_clear(a) be a double free?

[roed]

I've addressed the concerns about sig_on and the Cython for loops. I agree that the super-nested try blocks are ugly, but I don't know how to rewrite it safely without this nesting.

[jpflori]

Replying to roed:

What about things like mpz_clear, fmpz_clear and fmpz_poly_clear? If a is an fmpz_poly_t that has not yet had fmpz_poly_init(a) run on it, won't fmpz_poly_clear(a) be a double free?

Yes, they will and Sage will crash, or at least could. On Linux you can set _MALLOC_CHECK=3 to ensure (or have a higher probability) it will crash in such cases.

[roed]

Replying to jpflori:

Replying to roed:

What about things like mpz_clear, fmpz_clear and fmpz_poly_clear? If a is an fmpz_poly_t that has not yet had fmpz_poly_init(a) run on it, won't fmpz_poly_clear(a) be a double free?

Yes, they will and Sage will crash, or at least could. On Linux you can set _MALLOC_CHECK=3 to ensure (or have a higher probability) it will crash in such cases.

So, does anyone have suggestions on how to rewrite the ugly nested try blocks in PowComputer_flint_unram.__cinit__? Julian and I couldn't think of a better way to do it that would never have a memory leak or a double free.

[jpflori]

I'll have a look tomorrow (french time).

[jdemeyer]

Replying to roed:

So, does anyone have suggestions on how to rewrite the ugly nested try blocks in PowComputer_flint_unram.__cinit__? Julian and I couldn't think of a better way to do it that would never have a memory leak or a double free.

I had a look at the code and the current code actually is never going to work. The C-level functions like fmpz_poly_init and mpz_init never raise exceptions. So, the try/except is pointless. As far as I can tell, there are two kinds of exceptions which can happen: a MemoryError in MPIR or a KeyboardInterrupt. In both cases, the exception will be raised by sig_on() and none of the except blocks are executed.

If you want to be absolutely correct, every single allocation should be wrapped individually in sig_on()/sig_off(). Example for 2 allocations:

sig_on()
alloc1()
sig_off()

try:
    sig_on()
    alloc2()
    sig_off()
except BaseException:
    free1()
    raise

But at least the very deeply nested block on this ticket is allocating extremely little memory (surely less than 1000 bytes), so why not simply

alloc1()
alloc2()

[saraedum]

Replying to jdemeyer:

Replying to roed:

So, does anyone have suggestions on how to rewrite the ugly nested try blocks in PowComputer_flint_unram.__cinit__? Julian and I couldn't think of a better way to do it that would never have a memory leak or a double free.

I had a look at the code and the current code actually is never going to work. [...]

You are right. I had misunderstood how sig_on/sig_off worked.

But at least the very deeply nested block on this ticket is allocating extremely little memory (surely less than 1000 bytes), so why not simply

alloc1()
alloc2()

Is this acceptable? In the unlikely case that this allocation fails because we're out of memory, then sage is going to crash right?

[jdemeyer]

Replying to saraedum:

Is this acceptable? In the unlikely case that this allocation fails because we're out of memory, then sage is going to crash right?

Well, at least the very deeply nested allocation in this branch is allocating very little memory. If that already fails, bad things are going to happen anyway.

[git]

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

​4f04d28	Fix PowComputer_flint_unram.__cinit__ to not leak memory when interrupted.
​60975a4	Remove the 11-nested try blocks in the interest of code readability, sacrificing some memory-leak resistance

[roed]

Here are two commits; the first has the 11-nested try blocks with "safe" memory handling, and the second removes them for code readability. I'm fine with either approach.

[jdemeyer]

This comment is wrong:

While the following allocations have the potential to leak
small amounts of memory when interrupted or when one of the
init methods raises a MemoryError

If you don't have sig_on(), the code cannot be interrupted (unless it's called from within sig_on(), but that would not be a good idea). It also cannot raise a MemoryError, since there is no sig_on() to raise them. Instead of a MemoryError, Sage would simply crash hard.

[roed]

Replying to jdemeyer:

If you don't have sig_on(), the code cannot be interrupted (unless it's called from within sig_on(), but that would not be a good idea). It also cannot raise a MemoryError, since there is no sig_on() to raise them. Instead of a MemoryError, Sage would simply crash hard.

So the whole block of allocations should be wrapped in a single sig_on()/sig_off()?

[jdemeyer]

Replying to roed:

So the whole block of allocations should be wrapped in a single sig_on()/sig_off()?

In that case, memory leaks are possible and the comment would be 100% correct.

[jpflori]

I see that there is some smartness added to sage_mpir_malloc/... functions in src/c_lib/memory.c, and these functions are hopefully called when mpir_init is (instead of plain malloc/...).

But as far as I now, nothing similar was done for flint, and if that's the case, a failed memory allocation would go undetected anyway (and Sage crash quite quickly with null pointer dereference or similar delicious stuff).

I just looked at flint memory allocation code (flfint_malloc/...), and a failed allocation by malloc would call the flint_memory_error function which calls abort().

[jdemeyer]

Replying to jpflori:

I just looked at flint memory allocation code (flfint_malloc/...), and a failed allocation by malloc would call the flint_memory_error function which calls abort().

That's great because (unlike MPIR which silenty ignores memory errors) an abort() within sig_on() yields a RuntimeError which can safely be handled.

[pbruin]

Why not increment the __allocated counter after every successful allocation and use its value in __dealloc__() to decide which objects have to be deallocated? E.g.

def __cinit__(self, ...):
    sig_on()
    self.__allocated = 0
    fmpz_init(self.fmpz_ccmp); self.__allocated++
    fmpz_init(self.fmpz_cval); self.__allocated++
    ...
    fmpz_poly_init(self.poly_cconv); self.__allocated++
    fmpz_poly_init(self.poly_ctm); self.__allocated++
    ...
    sig_off()

def __dealloc__(self, ...):
    if self.__allocated >= 1:
        fmpz_clear(self.fmpz_ccmp)
    if self.__allocated >= 2:
        fmpz_clear(self.fmpz_cval)
    ...

I hope I'm understanding the problem correctly; I'm just looking at the code and didn't follow the above discussion very closely.

[jdemeyer]

For that to work properly, self.__allocated should have type volatile int.

[pbruin]

Replying to jdemeyer:

For that to work properly, self.__allocated should have type volatile int.

OK, I can believe that. Does Cython support volatile?

[jdemeyer]

Replying to pbruin:

Replying to jdemeyer:

For that to work properly, self.__allocated should have type volatile int.

OK, I can believe that. Does Cython support volatile?

Not directly, as far as I know. You can do

cdef extern from *:
    ctypedef int volatile_int "volatile int"

and then use volatile_int.

[vbraun]

Agree with the volatile being necessary so that the optimizer doesn't combine the increments into a single += n. But afaik it would still be legal for the optimizer to reorder statements as

    self.__allocated++
    self.__allocated++
    ...
    self.__allocated++
    fmpz_init(self.fmpz_ccmp)
    fmpz_init(self.fmpz_cval)
    ...
    fmpz_poly_init(self.poly_ctm)

So either mark everything that needs to be accessed in-order as volatile. Or use the usual pattern

def __cinit__(self, ...):
    self.fmpz_ccmp = NULL
    self.fmpz_cval = NULL
    sig_on()
    fmpz_init(self.fmpz_ccmp)
    fmpz_init(self.fmpz_cval)
    sig_off()

def __dealloc__(self, ...):
    if self.fmpz_ccmp != NULL:
        fmpz_clear(self.fmpz_ccmp)
    if self.fmpz_ccval != NULL:
        fmpz_clear(self.fmpz_ccval)

[vbraun]

I guess that can still be broken by creative reordering of the pointer initialization, so to be 100% safe one would need to split the setting to NULL from the fmpz_init either by

1. a compiler barrier (asm volatile("" ::: "memory") in gcc)
2. move the fmpz_init into __init__
3. RAII (separate classes for each fmpz_poly_t)

[git]

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

​cda1d75

Wrap allocations in sig_on/sig_off

[pbruin]

I merged the latest development branch (conflicts in sage/rings/padics/factory.py and sage/rings/padics/pow_computer.pyx), and then got this error while compiling:

Error compiling Cython file:
------------------------------------------------------------
...

        fmpz_poly_content(prime_pow.fmpz_cinv, a)
        fmpz_poly_scalar_divexact_fmpz(out, a, prime_pow.fmpz_cinv)

        fmpz_poly_xgcd(prime_pow.fmpz_cinv2, out, prime_pow.poly_cinv2, out, prime_pow.poly_cinv)
        if fmpz_is_zero(prime_pow.cinv2): raise ValueError("polynomials are not coprime")
                                ^
------------------------------------------------------------

./sage/libs/linkages/padics/fmpz_poly_unram.pxi:343:33: Cannot convert Python object to 'fmpz_t'

[saraedum]

I'm happy with the changes roed made since my last positive_review. So from my point of view this could go back to positive_review once somebody reviewed my few changes and if the patchbot is happy.

---

New commits:

​f11fb44	Merge branch 'develop' into ticket/14304
​cce5076	adapt to the new version of FLINT's padic_ctx_init
​994be79	Fixed a typo in linkage to FLINT for qadics
​c4348c2	fixed a doctest in padic factor

[git]

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

​d4d1353	Remove some trailing whitespace
​13902d3	Fix copy() for qadic coercion/conversion maps
​3c1d9ad	Add __richcmp__ for qadic element classes
​5b42f9b	Fixed doctests.

[saraedum]

I hope that this fixes all the bugs the patchbot detected.

[git]

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

​2fcdde8	Fix copy() for qadic coercion maps from capped absolute rings
​f33bd5b	Minor change to exception handling in MatrixSpace.matrix()

[git]

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

​c7ffd25

Fix copy() for qadic conversion maps to capped absolute rings

[chapoton]

needs rebase

[git]

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

​9fecbdd

Merge branch 'develop' into t/14304/ticket/14304

[git]

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

​5ba4124

Merge branch 'develop' into t/14304/ticket/14304

[git]

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

​e6bc498	fixed some hashing doctests of q-adics
​5e17cdb	Do not rely on generic conversion to Rational when converting from finite fields to p-adics

[git]

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

​2c3a710

implement _cache_key for q-adics

[jdemeyer]

Just a few technical comments (I don't plan to formally review this):

1. You don't need to add include_dirs = [SAGE_INC + '/flint']
2. You don't need to add __richcmp__ calling _richcmp
3. You don't need to add __hash__ which just raises TypeError: all Elements are now unhashable by default.
4. For consistency with other .pxd files, could you move all declarations of FLINT types to sage/libs/flint/types.pxd?
5. In this block

               try:
                   padic_ctx_init(self.ctx, self.fprime, 0, prec_cap, PADIC_SERIES)
               except:
                   mpz_clear(self.top_power)
                   raise
   

given that padic_ctx_init is a C function, how could it ever raise a Python exception? Moreover, I find it bad style to use except:, you catch be more explicit and catch except BaseException: instead (the reason I say this is because most people write except: by mistake).

[git]

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

​ab07bc4

remove unnecessary include_dirs for FLINT from module_list.py

[git]

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

​52425be	Merge branch 'develop' into t/14304/ticket/14304
​2a1f293	remove unnecessary implementation of __hash__ and __richcmp__
​6fbb0d7	fix conversion from residue field to q-adic fields
​70c780d	Merge branch 'develop' into t/14304/ticket/14304

[git]

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

​0ab73f6

Convert to p-adic residue field if there is a coercion

[jdemeyer]

Why this change?

src/sage/algebras/quatalg/quaternion_algebra_element.pyx

@@ -18 +18 @@
 #                  http://www.gnu.org/licenses/
 #*****************************************************************************
 
+from sage.libs.flint.fmpz cimport *
+from sage.libs.flint.fmpz_poly cimport *
+from sage.libs.flint.ntl_interface cimport *
 from sage.structure.element cimport AlgebraElement, RingElement, ModuleElement, Element
 from sage.algebras.quatalg.quaternion_algebra_element cimport QuaternionAlgebraElement_abstract
 from sage.rings.rational cimport Rational

[jdemeyer]

There are also some strange added imports in src/sage/rings/polynomial/...

[jdemeyer]

Similarly:

src/sage/libs/flint/ntl_interface.pxd

@@ -2 +2 @@
 
 from sage.libs.ntl.ntl_ZZ_decl cimport ZZ_c
 from sage.libs.ntl.ntl_ZZX_decl cimport ZZX_c
+from sage.libs.flint.fmpz cimport fmpz_t
+from sage.libs.flint.fmpz_poly cimport fmpz_poly_t
 
 cdef extern from "flint/NTL-interface.h":
     void fmpz_poly_get_ZZX(ZZX_c output, fmpz_poly_t poly)

[saraedum]

These imports happened because we fixed flint build errors by importing the necessary symbols. We just imported them in different places than what got into develop did. So the merge did not catch this conflict it seems. Thanks for pointing these out.

[git]

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

​63d90c6

remove duplicate imports

[git]

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

​932d5dd	Revert "remove unnecessary implementation of __hash__ and __richcmp__"
​94dda77	remove unnecessary implementation of __richcmp__

[git]

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

​d2ced66

Fix conversion from finite rings to q-adic fields

[jdemeyer]

Conflicts with 6.10.beta6. If you fix this conflict, I can easily merge #19646.

[git]

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

​1238235

Merge branch 'develop' into t/14304/ticket/14304

[git]

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

​3a68eb5	Merge branch 'develop' of git://trac.sagemath.org/sage into t/14304/ticket/14304
​33df900	fix build errors in padics/pow_computer_flint

[git]

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

​99fea64	silence deprecation warning in padics factory
​b522beb	fixed incorrect doctests related to p-adic hashing
​73303b8	fixed identity morphism for unramified p-adics

[saraedum]

All doctests in rings/ now pass (have not checked with --long.)

[git]

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

​89a56a0	implement residue() for q-adic elements
​12c7cc3	Fix p-adic doctests
​8291b03	implement conversion from polynomials to q-adics

[jdemeyer]

See 63

[jdemeyer]

Avoid cdefs.pxi, just cimport whatever you need.

[git]

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

​e376e21	Merge branch 'develop' into t/14304/ticket/14304
​860366d	remove except: for C function call in pow_computer
​4b33f86	fix build error

[saraedum]

Replying to jdemeyer:

1. You don't need to add include_dirs = [SAGE_INC + '/flint'] (just add flint/ to the path to the .h files)

Fixed in ​ab07bc41a76f58c6063c5e7a9ab8b4add7bc4f6a.

2. You don't need to add __richcmp__ calling _richcmp

Fixed in ​2a1f29345297da21ffe14c47e39faa1ac76eda5a.

3. You don't need to add __hash__ which just raises TypeError: all Elements are now unhashable by default.

I actually do need this implementation to override the base class. Adressed in ​2a1f29345297da21ffe14c47e39faa1ac76eda5a.

4. For consistency with other .pxd files, could you move all declarations of FLINT types to sage/libs/flint/types.pxd?

I believe that this has happened by now.

5. In this block

               try:
                   padic_ctx_init(self.ctx, self.fprime, 0, prec_cap, PADIC_SERIES)
               except:
                   mpz_clear(self.top_power)
                   raise
   

given that padic_ctx_init is a C function, how could it ever raise a Python exception? Moreover, I find it bad style to use except:, you should be more explicit and catch except BaseException: instead (the reason I say this is because most people write except: by mistake).

Both fixed in ​860366d0bf36e8684c97f5bd5d3e336e79f5795d.

---

New commits:

​e376e21	Merge branch 'develop' into t/14304/ticket/14304
​860366d	remove except: for C function call in pow_computer
​4b33f86	fix build error

[saraedum]

Replying to jdemeyer:

Avoid cdefs.pxi, just cimport whatever you need.

Fixed.

[git]

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

​e8c1e41

remove cdefs.pxi imports in qadics

[chapoton]

I just corrected a raise statement. There seems to be many failing doctests.

---

New commits:

​052d171	Merge branch 'u/saraedum/ticket/14304' into 7.1
​87ee786	trac #14304 correct one raise into py3 format, plus some doc correction

[git]

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

​c3b41a4	Merge branch 'develop' into t/14303/ticket/14304
​2141bd9	Fix bug that would be triggered for ramified extensions
​9c4aecb	Improve docstring to use .. WARNING
​354bc5e	Break up (some of) the long lines
​b422cfc	Merge branch 'public/14304' of git://trac.sagemath.org/sage into t/14304/ticket/14304
​26c8669	Fix typo in pow computer

[aly.deines]

On line 494 in sage/local/lib/python2.7/site-packages/sage/rings/padics/factory.py, you need to break the line using parenthesis.

aly@aly-laptop:~/Sage/sage$ ./sage -t src/sage
Traceback (most recent call last):
  File "/home/aly/Sage/sage/src/bin/sage-runtests", line 80, in <module>
    from sage.doctest.control import DocTestController
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/control.py", line 30, in <module>
    from sources import FileDocTestSource, DictAsObject
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/sources.py", line 27, in <module>
    from parsing import SageDocTestParser
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/parsing.py", line 49, in <module>
    from sage.all import RealIntervalField
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/all.py", line 100, in <module>
    from sage.rings.all      import *
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/rings/all.py", line 62, in <module>
    from padics.all import *
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/rings/padics/all.py", line 2, in <module>
    from factory import Zp, Zq, Zp as pAdicRing, ZpCR, ZpCA, ZpFM, ZqCR, ZqCA, ZqFM #, ZpL, ZqL
  File "/home/aly/Sage/sage/local/lib/python2.7/site-packages/sage/rings/padics/factory.py", line 494
    if version[0] < 4 or (len(version) > 1 and version[0] == 4 and version[1] < 5) or
                                                                                    ^
SyntaxError: invalid syntax

[git]

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

​baca192

Add parentheses for line wraps in factory.pyx

[roed]

Fixed

[jdemeyer]

Needs to be rebased on top of #20210.

[git]

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

​03458ea	Upgrade cysignals package
​dce67fc	Move memory functions to cysignals
​4bb8337	Rename sage_malloc -> sig_malloc and friends
​c22ae81	Merge commit '4bb8337295ed82c93d1a9c9f7173a4c3' into t/14304/ticket/14304

[git]

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

​a14ee5d

Fixing imports and name changes due to switch to cysignals

[roed]

Okay, I think I rebased it, but there are a ton of doctest errors that have nothing to do with p-adics. I'm not sure what's going on.

sage -t src/sage/combinat/root_system/root_lattice_realizations.py  # 1 doctest failed
sage -t src/sage/combinat/designs/database.py  # 2 doctests failed
sage -t src/sage/geometry/polyhedron/base.py  # 2 doctests failed
sage -t src/sage/graphs/generic_graph.py  # 94 doctests failed
sage -t src/sage/graphs/graph.py  # 46 doctests failed
sage -t src/sage/doctest/forker.py  # 1 doctest failed
sage -t src/sage/schemes/generic/algebraic_scheme.py  # 10 doctests failed
sage -t src/sage/graphs/generators/smallgraphs.py  # 18 doctests failed
sage -t src/sage/tests/cmdline.py  # 14 doctests failed
sage -t src/sage/structure/sage_object.pyx  # 4 doctests failed
sage -t src/sage/combinat/rigged_configurations/rigged_configuration_element.py  # 1 doctest failed
sage -t src/sage/schemes/toric/points.py  # 1 doctest failed
sage -t src/sage/combinat/posets/posets.py  # 5 doctests failed
sage -t src/sage/interfaces/sage0.py  # 1 doctest failed
sage -t src/sage/combinat/designs/orthogonal_arrays.py  # 8 doctests failed
sage -t src/sage/graphs/generators/families.py  # 4 doctests failed
sage -t src/sage/geometry/cone.py  # 3 doctests failed
sage -t src/sage/combinat/rigged_configurations/rigged_configurations.py  # 1 doctest failed
sage -t src/sage/combinat/designs/orthogonal_arrays_build_recursive.py  # 13 doctests failed
sage -t src/sage/matroids/matroid.pyx  # 1 doctest failed
sage -t src/sage/repl/interpreter.py  # 22 doctests failed
sage -t src/sage/structure/misc.pyx  # 1 doctest failed
sage -t src/sage/combinat/designs/incidence_structures.py  # 10 doctests failed
sage -t src/sage/numerical/optimize.py  # 6 doctests failed
sage -t src/sage/combinat/rigged_configurations/rc_infinity.py  # 1 doctest failed
sage -t src/sage/graphs/comparability.pyx  # 6 doctests failed
sage -t src/sage/graphs/generators/basic.py  # 1 doctest failed
sage -t src/sage/repl/ipython_extension.py  # 9 doctests failed
sage -t src/sage/repl/attach.py  # 2 doctests failed
sage -t src/sage/graphs/digraph.py  # 4 doctests failed
sage -t src/sage/typeset/ascii_art.py  # 2 doctests failed
sage -t src/sage/numerical/mip.pyx  # 486 doctests failed
sage -t src/sage/tests/french_book/nonlinear_doctest.py  # 1 doctest failed
sage -t src/sage/interacts/debugger.py  # 1 doctest failed
sage -t src/sage/repl/display/formatter.py  # 10 doctests failed
sage -t src/sage/combinat/designs/bibd.py  # 19 doctests failed
sage -t src/sage/repl/inputhook.pyx  # 2 doctests failed
sage -t src/sage/misc/temporary_file.py  # 1 doctest failed
sage -t src/sage/doctest/test.py  # 1 doctest failed
sage -t src/sage/repl/interface_magic.py  # 3 doctests failed
sage -t src/sage/dev/trac_interface.py  # 1 doctest failed
sage -t src/doc/en/thematic_tutorials/lie/infinity_crystals.rst  # 1 doctest failed
sage -t src/sage/combinat/posets/poset_examples.py  # 1 doctest failed
sage -t src/sage/repl/ipython_tests.py  # 4 doctests failed
sage -t src/sage/combinat/designs/orthogonal_arrays_find_recursive.pyx  # 2 doctests failed
sage -t src/sage/graphs/graph_decompositions/vertex_separation.pyx  # 7 doctests failed
sage -t src/sage/numerical/backends/glpk_backend.pyx  # 470 doctests failed
sage -t src/sage/combinat/integer_vector.py  # 5 doctests failed
sage -t src/sage/numerical/linear_functions.pyx  # 212 doctests failed
sage -t src/sage/combinat/rigged_configurations/rc_crystal.py  # 1 doctest failed
sage -t src/sage/combinat/shard_order.py  # 1 doctest failed
sage -t src/sage/graphs/generators/degree_sequence.py  # 3 doctests failed
sage -t src/sage/repl/rich_output/backend_ipython.py  # 3 doctests failed
sage -t src/sage/numerical/linear_tensor_element.pyx  # 50 doctests failed
sage -t src/doc/en/thematic_tutorials/numerical_sage/weave.rst  # 1 doctest failed
sage -t src/sage/numerical/linear_tensor_constraints.py  # 46 doctests failed
sage -t src/sage/numerical/backends/cvxopt_backend.pyx  # 10 doctests failed
sage -t src/sage/numerical/linear_tensor.py  # 57 doctests failed
sage -t src/sage/repl/ipython_kernel/install.py  # 1 doctest failed
sage -t src/sage/graphs/generators/chessboard.py  # 1 doctest failed
sage -t src/doc/en/reference/sat/index.rst  # 4 doctests failed
sage -t src/sage/numerical/backends/generic_backend.pyx  # 1 doctest failed
sage -t src/sage/graphs/graph_coloring.py  # 17 doctests failed
sage -t src/sage/numerical/knapsack.py  # 5 doctests failed
sage -t src/sage/graphs/hypergraph_generators.py  # 1 doctest failed
sage -t src/sage/repl/ipython_kernel/kernel.py  # 4 doctests failed
sage -t src/doc/en/thematic_tutorials/linear_programming.rst  # 35 doctests failed
sage -t src/sage/sat/solvers/sat_lp.py  # 15 doctests failed
sage -t src/sage/graphs/graph_decompositions/cutwidth.pyx  # 9 doctests failed
sage -t src/sage/graphs/convexity_properties.pyx  # 4 doctests failed
sage -t src/sage/sat/solvers/satsolver.pyx  # 1 doctest failed
sage -t src/sage/numerical/backends/gurobi_backend.pyx  # 1 doctest failed

[git]

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

​56b357c

Fixing some more pxi includes

[roed]

Ok, I think I fixed the problems.

[aly.deines]

I'm getting the same list of doctests failing. They don't fail on #20210. This is weird. I did a clean pull and tested them. None of them seem to have anything to do with your patch. Are they passing for you?

[roed]

I think mine are passing. See Prec_Build.term on the project....

[aly.deines]

I updated my development branch and now everything is passing. It looks good to me! :)

[saraedum]

(I was told to be consistent about my last name…)