Nemo.jl icon indicating copy to clipboard operation
Nemo.jl copied to clipboard

Added new fn is_zero_det_probabilistic

Open JohnAAbbott opened this issue 2 months ago • 6 comments

New heuristic function is_zero_det_probabilistic; incl. doc & tests.

JohnAAbbott avatar Oct 13 '25 12:10 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.

JohnAAbbott avatar Oct 13 '25 12:10 JohnAAbbott

Codecov Report

:x: Patch coverage is 88.63636% with 5 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 88.21%. Comparing base (178d272) to head (46f41aa).

Files with missing lines Patch % Lines
src/flint/fmpq_mat.jl 79.16% 5 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2152   +/-   ##
=======================================
  Coverage   88.20%   88.21%           
=======================================
  Files          99       99           
  Lines       37079    37123   +44     
=======================================
+ Hits        32707    32747   +40     
- Misses       4372     4376    +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Oct 13 '25 12:10 codecov[bot]

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?

thofma avatar Oct 13 '25 14:10 thofma

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.

JohnAAbbott avatar Oct 13 '25 14:10 JohnAAbbott

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

thofma avatar Oct 13 '25 14:10 thofma

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.

JohnAAbbott avatar Oct 29 '25 14:10 JohnAAbbott