flintlib / flint

FLINT (Fast Library for Number Theory)
http://www.flintlib.org
GNU Lesser General Public License v3.0
401 stars 235 forks source link

Bug in `fq_nmod_mpoly_factor` triggered by large test multiplier #1978

Open albinahlback opened 1 month ago

albinahlback commented 1 month ago

The tests for fq_nmod_mpoly_factor fails via FLINT_ASSERT:

fq_nmod_mpolyu_set: Assertion `fq_nmod_mpolyu_is_canonical(B, uctx)' failed

This is present in the main branch, version 3.0.0 and possibly earlier as well. To trigger it, run current test for fq_nmod_mpoly_factor with assertion enabled and FLINT_TEST_MULTIPLIER greater or equal to 6.

Edit: One has to alter the test code slightly for 3.0.0 to trigger it. Specifically, one has to use Fredrik's code from this commit to trigger it.

fredrik-johansson commented 1 month ago

I think I found the cause. In fq_nmod_mpolyu_gcds_zippel, the polynomial f is a deliberately non-canonicalised "form" polynomial containing the sought exponents but with zero coefficients.

The following code

        FLINT_ASSERT((f->coeffs + 0)->length == 1);
        fq_nmod_mpolyu_set(G, f, ctx);
        _n_fq_one((G->coeffs + 0)->coeffs + d*0, d);

sets G to a valid the length-1 polynomial with form f and coefficient 1 instead of 0, but the assert in fq_nmod_mpolyu_set triggers before the coefficient is assigned.

I believe replacing fq_nmod_mpolyu_set by fq_nmod_mpolyu_setform solves the problem.

Not a serious bug, because fq_nmod_mpolyu_set happens to do the thing when the input is non-canonical, but ought to be fixed for cleanliness.

@tthsqe12 does that look correct?

It looks like we have the same issue in src/nmod_mpoly/mpolyu_gcdp_zippel.c, but that case isn't reached by tests: https://app.codecov.io/gh/flintlib/flint/blob/main/src%2Fnmod_mpoly%2Fmpolyu_gcdp_zippel.c#L231

While we're at it, it would be nice to improve the test code in nmod_mpoly so that we have coverage there.

The GCD code for fmpz_mod_mpoly looks a different; not sure if it has the same problem.

fredrik-johansson commented 1 month ago

I've tried to tweak the nmod_mpoly_gcd_zippel test code (generating polynomials with fewer variables and terms) to hit that branch in nmod_mpolyu_gcds_zippel, but I've been unable to reach it. I wonder if it's actually reachable? Perhaps one has to test nmod_mpolyu_gcds_zippel directly.