Test failure in sage.rings.complex_arb on Cygwin

[embray]

File "src/sage/rings/complex_arb.pyx", line 789, in sage.rings.complex_arb.ComplexBallField.?
Failed example:
    pol.roots(multiplicities=False, proof=False)
Expected:
    doctest:...
    UserWarning: roots may have been lost...
    [[0.001000000 +/- 2.52e-10] + [+/- 2.05e-10]*I,
     [0.00100000 +/- 1.56e-10] + [+/- 1.27e-10]*I,
     [18.20524201487994 +/- 1.22e-15] + [+/- 5.75e-37]*I,
     [-14.72907378354557 +/- 4.63e-15] + [10.70100790294238 +/- 2.16e-15]*I,
     [-14.72907378354557 +/- 4.63e-15] + [-10.70100790294238 +/- 2.16e-15]*I,
     [5.625452776105595 +/- 2.29e-16] + [17.31459450084417 +/- 4.09e-15]*I,
     [5.625452776105595 +/- 2.29e-16] + [-17.31459450084417 +/- 4.09e-15]*I]
Got:
    doctest:warning
...
    :
    UserWarning: roots may have been lost
    [[0.001000000 +/- 2.52e-10] + [+/- 2.05e-10]*I,
     [0.00100000 +/- 1.56e-10] + [+/- 1.27e-10]*I,
     [18.20524201487994 +/- 1.22e-15] + [+/- 5.75e-37]*I,
     [-14.72907378354557 +/- 4.63e-15] + [10.70100790294238 +/- 2.16e-15]*I,
     [-14.72907378354557 +/- 4.63e-15] + [-10.70100790294238 +/- 2.16e-15]*I,
     [5.625452776105595 +/- 2.29e-16] + [-17.31459450084417 +/- 4.09e-15]*I,
     [5.625452776105595 +/- 2.29e-16] + [17.31459450084417 +/- 4.09e-15]*I]

The only difference (and I had to stare a while to see this) was in the sort order of the last 2 roots:

     [5.625452776105595 +/- 2.29e-16] + [17.31459450084417 +/- 4.09e-15]*I,
     [5.625452776105595 +/- 2.29e-16] + [-17.31459450084417 +/- 4.09e-15]*I

vs

[5.625452776105595 +/- 2.29e-16] + [-17.31459450084417 +/- 4.09e-15]*I,
[5.625452776105595 +/- 2.29e-16] + [17.31459450084417 +/- 4.09e-15]*I

where the only difference between the two is the sign on the complex part.

This failure, unsurprisingly, appears to be a possible problem with
arb; specifically the function _arb_vec_sort_pretty:
https://github.com/fredrik-johansson/arb/blob/f4dda0f34f30cb8cbfe3b18eafc9e5e8480a88da/acb/vec_sort_pretty.c#L48

I'm not sure why it only appears to be affecting me on Cygwin...

CC: @fredrik-johansson

Component: porting: Cygwin

Keywords: arb

Author: Marc Mezzarobba

Branch: badccf6

Reviewer: Travis Scrimshaw

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

[mezzarobba]

comment:1

qsort() is not guaranteed to be a stable sort. I guess the two roots compare equal under arb's comparison function, and the qsort() implementations don't sort them in the same order.

Fredrik, assuming this is correct, would it make sense to use a total order in vec_cort_pretty()? Or should we work around the issue in sage?

[fredrik-johansson]

comment:2

It makes sense to fix the comparison function.

[embray]

comment:3

Even so, it would be just as easy to supply a workaround in Sage as well. That said, perhaps the workaround is not high priority if the only affected platform is Cygwin. Perhaps the only reason it's affecting Cygwin is due to differences in the qsort implementation.

[RalphieBoy]

comment:4

FWIW, I am seeing this consistently (since 8.3-b0, including 8.3-b8) on macOS 10.11.6. If I hack the source to reverse the last two entries in the root list, tests pass.

[vbraun]

comment:5

Not returning the answer in the desired sort order isn't exactly blocker material. I'm happy to merge if you provide a timely fix, of courese.

[embray]

comment:6

I believe any failing test or regression should be considered a blocker. How can you guarantee useful continuous integration tests if every run fails? Do I have to check the test results every time to see if something other than a known failure failed?

If nothing else, until and unless Sage's test suite has a way of marking known failures to be ignored this should be a blocker.

[embray]

comment:7

Side note: it's one thing if it's just an occasional random failure if it's rare. It's another if something fails consistently on some platforms. A very simple fix to this would be to either change the code to provide additional sorting at the Python level, or change the test to not care (I'm fine with either). But unless it's passing it should be a blocker.

[mezzarobba]

Commit: badccf6

[mezzarobba]

Branch: u/mmezzarobba/ticket/25620

[mezzarobba]

Author: Marc Mezzarobba

[mezzarobba]

New commits:

badccf6

#25620 work around weakness of _arb_vec_sort_pretty in test

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:9

LGTM.

[vbraun]

Changed branch from u/mmezzarobba/ticket/25620 to badccf6

[embray]

comment:11

Thanks--such a fix is good enough for me.

[embray]

Changed commit from badccf6 to none