flintlib / python-flint

Python bindings for Flint and Arb
MIT License
130 stars 24 forks source link

factorisation in nmod_poly does not gracefully handle errors #73

Open GiacomoPope opened 1 year ago

GiacomoPope commented 1 year ago
>>> nmod_poly([12, 7, 1], 10)
x^2 + 7*x + 2
>>> nmod_poly([12, 7, 1], 10).factor()
Flint exception (Impossible inverse):
    Cannot invert modulo 2*0
zsh: abort      python3
oscarbenjamin commented 1 year ago

In general I think that python-flint should avoid segfaults and aborts (except perhaps if it exposes a very low-level API).

There are some cases like this though where it is tricky because how can we know ahead of time that this will operation will or will not succeed?

In [3]: nmod_poly([1, 1], 10).factor()
Out[3]: (1, [(x + 1, 1)])

If the modulus is prime it should always succeed. The Flint docs don't give any clue as to when this is expected to work or under what circumstances it might crash. At least python-flint should warn in its docs whether a particular function might abort because aborts are considered unfriendly in Python-land.

GiacomoPope commented 1 year ago

In SageMath a common trick is with the sig_on and sig_off to check for issues, we could do something like that here?

oscarbenjamin commented 1 year ago

check for issues

What check could be done besides just warning if the modulus is not prime?

GiacomoPope commented 1 year ago

Oh for the sig_on() and sig_off() I think it just prints the error from C without the abort? So you have:

sig_on()
... some dangerous code
sig_off()

If the modulus is prime, then every element is invertible, a softer check is just ensuring that there's no non-trivial gcd before an inversion is called?

However cysignals would become another requirement for python-flint and I don't know if this is wanted?

oscarbenjamin commented 1 year ago

Perhaps there could be an argument like .factor(check=True) or something.