Bug in `fq_nmod_mpoly_factor` triggered by large test multiplier
albinahlback opened this issue · 2 comments
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.
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.
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.