py3: fix output buffering issues in eclib

[jdemeyer]

There are doctest failures under Python 3 in eclib due to output from eclib being buffered instead of printed.

Python 2 uses the standard C library I/O functions from <stdio.h>. Typically, the C library implements buffering. Now Python 3 bypasses the C library for I/O and does direct system calls from <unistd.h>. The doctest framework always flushes the Python buffers after every test. Under Python 2, this actually flushes the <stdio.h> buffer which is the same buffer used by eclib (and other C/C++ libraries). But on Python 3, this only flushes the Python-specific buffer but not the <stdio.h> buffer.

CC: @fchapoton @JohnCremona

Component: python3

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

[jdemeyer]

comment:1

It would be possible to add flushing functions in eclib itself but that does not seem the right approach to me.

I would argue that eclib should actually flush whenever it's "done" producing output. Note that << endl flushes the output, so this is probably a matter of replacing some << "\n" by << endl in eclib, in particular in mw::process(). Alternatively, you could add an explicit cout.flush().

Note that flushing too often can degrade performance. But I guess that I/O performance is not an issue for eclib (you aren't producing megabytes of output per second, right?). So then there is little reason to not use << endl.

[JohnCremona]

comment:2

Replying to @jdemeyer:

It would be possible to add flushing functions in eclib itself but that does not seem the right approach to me.

I would argue that eclib should actually flush whenever it's "done" producing output. Note that << endl flushes the output, so this is probably a matter of replacing some << "\n" by << endl in eclib, in particular in mw::process(). Alternatively, you could add an explicit cout.flush().

Note that flushing too often can degrade performance. But I guess that I/O performance is not an issue for eclib (you aren't producing megabytes of output per second, right?). So then there is little reason to not use << endl.

Certainly I understand all that. The actual programs in eclib do this, but not all the library functions. I can work on that but surely (1) the recent eclib version can be merged without waiting for yet another one, and (2) the flushing of all output in doctests as proposed in this ticket should be done anyway?

[jdemeyer]

comment:3

Indeed, this shouldn't require a new eclib release.

Concretely for this ticket, I would propose something like

diff --git a/src/sage/libs/eclib/mwrank.pyx b/src/sage/libs/eclib/mwrank.pyx
index 5625153..e9d5b90 100644
--- a/src/sage/libs/eclib/mwrank.pyx
+++ b/src/sage/libs/eclib/mwrank.pyx
@@ -23,6 +23,7 @@ from __future__ import print_function, absolute_import
 import os
 import sys
 
+from libc.stdio cimport fflush, stdout
 from cysignals.memory cimport sig_free
 from cysignals.signals cimport sig_on, sig_off
 
@@ -726,6 +727,7 @@ cdef class _mw:
         sig_on()
         x,y,z = _bigint(point[0]), _bigint(point[1]), _bigint(point[2])
         r = mw_process(self.curve, self.x, x.x, y.x, z.x, sat)
+        fflush(stdout)
         sig_off()
         if r != 0:
             raise ArithmeticError("point (=%s) not on curve." % point)

Maybe this needs to be done in a few more places, but I really have no time to work on this now.

[jdemeyer]

comment:4

Replying to @JohnCremona:

the flushing of all output in doctests as proposed in this ticket should be done anyway?

I don't think that the doctest framework is the right place to fix this. We want to fix this also in normal interactive usage, not just in doctests.

[JohnCremona]

comment:5

Replying to @jdemeyer:

Indeed, this shouldn't require a new eclib release.

Concretely for this ticket, I would propose something like

diff --git a/src/sage/libs/eclib/mwrank.pyx b/src/sage/libs/eclib/mwrank.pyx
index 5625153..e9d5b90 100644
--- a/src/sage/libs/eclib/mwrank.pyx
+++ b/src/sage/libs/eclib/mwrank.pyx
@@ -23,6 +23,7 @@ from __future__ import print_function, absolute_import
 import os
 import sys
 
+from libc.stdio cimport fflush, stdout
 from cysignals.memory cimport sig_free
 from cysignals.signals cimport sig_on, sig_off
 
@@ -726,6 +727,7 @@ cdef class _mw:
         sig_on()
         x,y,z = _bigint(point[0]), _bigint(point[1]), _bigint(point[2])
         r = mw_process(self.curve, self.x, x.x, y.x, z.x, sat)
+        fflush(stdout)
         sig_off()
         if r != 0:
             raise ArithmeticError("point (=%s) not on curve." % point)

Maybe this needs to be done in a few more places, but I really have no time to work on this now.

Thanks for those hints. I'll see if I can make that work.

[jhpalmieri]

comment:6

Is this now fixed by #27360? With that ticket, Python 3 tests pass for me in libs/eclib.