Add new function is_probably_zero_det (for ZZMatrix, QQMatrix)

[JohnAAbbott]

New heuristic function is_zero_det_probabilistic; incl. doc & tests.
RENAMED later to is_probably_zero_det (see below)

[JohnAAbbott]

This is currently restricted to ZZMatrix. An extension to QQMatrix should be fairly easy. Extending to (univariate) polynomials over ZZ or QQ would take some more work, but not excessively much.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.34%. Comparing base (53a90bc) to head (37f4469).
⚠️ Report is 47 commits behind head on master.

Files with missing lines	Patch %	Lines
src/flint/fmpq_mat.jl	79.16%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2152      +/-   ##
==========================================
+ Coverage   88.30%   88.34%   +0.03%     
==========================================
  Files         100      100              
  Lines       37202    37290      +88     
==========================================
+ Hits        32853    32944      +91     
+ Misses       4349     4346       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thofma]

There is a lot of code duplication. How much slower is it to just do is_zero_det_probablisitic(numerator(x)) in the rational case?

[JohnAAbbott]

There is a lot of code duplication. How much slower is it to just do is_zero_det_probablistic(numerator(x)) in the rational case?

Yes, there is duplication, but I don't know how to avoid that without risking a significant speed penalty in some cases, e.g. the current test uses a matrix 250x250 whose entries are 1/k for k ranging from 1 to 250^2; this takes less than 0.02s on my computer, but computing the result by taking the numerator takes nearly 1.8s -- roughly 100 times slower.
OK, I could convert the ZZMatrix into a QQMatrix and then use the QQMatrix function... but that seems wasteful.

[thofma]

Thanks for checking. I agree that the overhead of converting to ZZMatrix is too large.

[JohnAAbbott]

Converting to a non-draft PR. The extensions mentioned in comment 2 (near the start of this discussion) can be dealt with separately if there is call for them.

[fingolfin]

tests whether "is_zero(det(M)) is probably true (if it returns true) and otherwise definitely false (if it returns false)

We have is_probable_prime so maybe this should be is_probably_zero_det (but sounds weird?).

Having it at end makes it easier to find, though.

---

Should this function be added to the documentation? It is currently being exported in this PR, which to me suggests it should be in the manual...

[lgoettgens]

Having it at end makes it easier to find, though.

I agree

[JohnAAbbott]

Is the release notes tag correct for this PR? I think so, but am not sure.

[lgoettgens]

Is the release notes tag correct for this PR? I think so, but am not sure.

yes, looks good

[JohnAAbbott]

I believe this is ready to merge (assuming we want the new function, and are happy with the name).

[fingolfin]

Note: we already have is_probable_prime and is_probable_supersingular. It is somewhat unfortunate that the names are different, but I am not sure that matters much.

The name is_probable_prime is justified by the existence of the term "probable prime".

So from my point of view, we could merge this as-is. If we later are concerned about this kind of consistency, we could add aliases?

Here at triage, nobody voiced a strong opinion...