nmod_poly_sqrt: throw error for non-prime modulus#2537
nmod_poly_sqrt: throw error for non-prime modulus#2537cxzhong wants to merge 3 commits intoflintlib:mainfrom
Conversation
The nmod_poly_sqrt, nmod_poly_sqrt_series, and nmod_poly_invsqrt_series functions only work correctly for prime moduli because: 1. n_sqrtmod uses Tonelli-Shanks algorithm (requires prime) 2. gr_sqrt returns GR_UNABLE for non-fields 3. Division by 2 (gr_mul_2exp_si) fails for even moduli 4. Newton iteration requires invertible elements Previously, using these functions with composite moduli like 16 would cause a crash with an uninformative GR_MUST_SUCCEED failure message. Now they throw a clear error: 'Modulus must be prime.' Fixes flintlib#2508
|
This is quite an expensive test. I think the usual flint philosophy is to state in the documentation that the modulus must be prime. If it is not prime, then "garbage in, garbage out" applies. |
Thank you. I will close that. Then I think we need to write this in document. But I also do not see this in document. |
|
Let's wait for some of the maintainers to chime in. |
|
As the primality test indeed can be relatively expensive, it would be better to try to compute the square root, and catch when this fails. If into this (warning: untested): Then I'd guess you'd also need to intercept failure of the Even more user-friendly would be to fix For the functions wrapping to throw a more informative error, e.g.: (Optionally, do some more checks when the computation actually failed to make the error message more specific.) Actually, These functions are really a bit of a mess at the moment; there's lots of things to clean up and optimize... And as you noted in #2508, it would be nice if |
|
In this case, return 0 means there is no sqrt root or we can not find a root via the algorithm, I think we should write in the document |
|
for odd composite numbers, it will return 0 when they have a sqrt root but the algorithm can not find |
|
@fredrik-johansson please review this |
|
If we do not do primality test, we can not exacly know it is a prime number or not |
Maybe we should write in the document, the modulus must be prime |
The
nmod_poly_sqrt,nmod_poly_sqrt_series, andnmod_poly_invsqrt_seriesfunctions only work correctly for prime moduli because:n_sqrtmoduses Tonelli-Shanks algorithm (requires prime)gr_sqrtreturns GR_UNABLE for non-fieldsgr_mul_2exp_si) fails for even moduliPreviously, using these functions with composite moduli like 16 would cause a crash with an uninformative
GR_MUST_SUCCEEDfailure message.Now they throw a clear error: 'Modulus must be prime.'
Fixes #2508