py3: numerous additional fixes for sage.numerical

[embray]

New commits:

​f08fb45	py3: numerous string encode/decode fixes for sage.numerical.backends
​9db2b12	Change the add_col method of backends to accept any iterable instead of strictly list.
​0b735bf	Python 3 fixes for the LoggingBackend
​49846ed	Support for tuple unpacking in function signatures was removed in Python 3
​14f355b	py3: miscellaneous minor Python 3 fixes and other cleanup, fixing all the tests for this package on Python 3
​3cbd09c	For some reason this deprecation warning is shown on Python 2 but not on Python 3

[embray]

I believe this issue can reasonably be addressed for Sage 8.4.

[chapoton]

branch is red

[embray]

Retargeting some of my tickets (somewhat optimistically for now).

[chapoton]

red branch

[chapoton]

still red

[embray]

This branch is such a mess now. I think a lot of the things in it were already fixed. I'm trying to figure it out...

[chapoton]

yes, indeed. That's why I did not manage to merge or rebase it myself..

[embray]

Okay, I'm working on it. From what I can tell there are still a handful of fixes in here that haven't made it into the mainline yet.

[git]

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

​51b3890

Trac #24741: py3: miscellaneous minor fixes, mostly to sage.numerical backends

[git]

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

​bbfc308

Trac #24741: py3: miscellaneous minor fixes, mostly to sage.numerical backends

[embray]

I think this is fine now. With this, the only failures I'm seeing in sage.numerical are due to __round__.

[chapoton]

• some changes will likely conflict with #27342 (sorry for the lack of coordination)

• I am worried about the changes in src/sage/numerical/backends/cvxopt_backend.pyx,

because we iterate twice over the input, and this will fail if it is an iterator..

[embray]

It could check if it's not already a list and just wrap it in list if not. That would be superfluous for other sized sequences but there's no better way to deal with that case without #26769, which I still contend is useful.

[embray]

Also if that's definitely a problem it's not caught be any tests. Why don't you see if it actually does break, and if so a regression test should be added.

[chapoton]

ok, then. Let us keep this question about list for somewhere else.

What about the conflict with #27342 ?

[dcoudert]

• why using if type(indices) is not list: instead of if not isinstance(indices, list): ?

• in src/sage/numerical/interactive_simplex_method.py

  -        names.extend([str(s) for s in self.x()])
  +        names.extend(str(s) for s in self.x())
  

otherwise, LGTM

[embray]

We want to convert these inputs explicitly to PyListType-typed objects so that Cython can generate efficient code to loop over them. This is why the previous code explicitly typed those arguments as list. This does effectively the same thing but allows the arguments to be iterables of other types, and then convert them to list.

[dcoudert]

I perfectly understood that you now allow to give an iterable as input, and that it is important to cast to PyListType.

What I don't understand is the use of type(indices) is not list instead of isinstance(indices, list). Is this a more efficient formulation in Cython or just a stylistic preference ?

[embray]

Yes, if you want to check an exact type using type(foo) is bar is orders of magnitude faster, because it's just a pointer comparison.

isinstance(...) is a much more complex beast that needs to be able to support things like subclasses as well as __instancecheck_ and __subclasscheck__. So it's better, especially in Cython code, to do exact type comparisons when that's all you need in a particular case.

[dcoudert]

Thanks for the explanation. It's good to know.

The remaining issues with py3 are due to __round__, and there is this possible minor improvement #comment:19 for names.extend.

[embray]

Hmm, not that it really matters much here either way but I'm not sure comment:19 is necessarily much of an improvement. In most cases I do advocate for generator expressions, but for small collections there's actually more overhead in terms of setting up the generator object and iterating over it than it's worth. For example:

In [1]: x = list(range(4))

In [2]: %%timeit
   ...: names = []
   ...: names.extend(str(_) for _ in x)
   ...:
100000 loops, best of 3: 2.19 µs per loop

In [3]: %%timeit
   ...: names = []
   ...: names.extend([str(_) for _ in x])
   ...:
1000000 loops, best of 3: 1.48 µs per loop

You can also see in the ​list.extend implementation that it has a more optimized code path for the case where it's given a list or tuple and can use the PySequence_Fast API, as opposed to the more generic path of having to guess what the new size of the list will be (using __length_hint__ if there is one) and slowly iterate over the iterable.

Of course that's all implementation detail and there's no guarantee in the language that this should be faster. By rights the iterator expression should be "better" but in practice it isn't, at least for small lists.

[dcoudert]

Thank you very much for this detailed explanation. Then I agree that current formulation is good.

To avoid merge conflict with #27342, you should revert some changes

• linear_functions.pyx (same done in #27342)
• mip.pyx:

  -            sage: x_sol.keys()
  +            sage: sorted(x_sol)
  

[embray]

Thanks for reminding me. Yes, I'll just rebase this on top of #27342.

[chapoton]

red branch, as expected

[embray]

All the better then.

[git]

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

​2ebacb6

Trac #24741: py3: miscellaneous minor fixes, mostly to sage.numerical backends

[embray]

I think this is still good, though I did not test on Python 3.

[tscrim]

With this branch on Python3, I go from

sage -t src/sage/numerical/mip.pyx  # 1 doctest failed
sage -t src/sage/numerical/backends/cvxopt_sdp_backend.pyx  # 6 doctests failed
sage -t src/sage/numerical/sdp.pyx  # 10 doctests failed
sage -t src/sage/numerical/backends/cvxopt_backend.pyx  # 9 doctests failed
sage -t src/sage/numerical/backends/glpk_backend.pyx  # Killed due to abort

to

sage -t src/sage/numerical/backends/cvxopt_backend.pyx  # 8 doctests failed
sage -t src/sage/numerical/backends/cvxopt_sdp_backend.pyx  # 5 doctests failed
sage -t src/sage/numerical/sdp.pyx  # 8 doctests failed

which all look like round-type failures

[dcoudert]

Same for me. Good to go.