-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing fmpz_mod_mpoly_compose_ functions #2068
Add missing fmpz_mod_mpoly_compose_ functions #2068
Conversation
Adds the documented, but unimplemented `fmpz_mod_mpoly_compose_fmpz_mod_mpoly_gen` function. Adds `fmpz_mod_mpoly_compose_horner` function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late reply.
There are some lines that are not covered by codecov. Almost all of these can be reached. Perhaps there are some hard-to-reach case, but we'd like to see 100% coverage.
80eeafa
to
978827e
Compare
I've added a test for aliased generated composition. If it hits the lines I'll also add it for the other types, they're missing it as well. But it looks like I forgot the zero case, I'll add this later. I'm not sure how to hit the missing line in |
d69fc35
to
fb0d20a
Compare
This should bring all generator composition functions to up 100% coverage. Should also improve the coverage for normal composition in the zero poly case. Not sure how they were bring hit previously, perhaps by the random generation, but now it's explicit. I'm not sure how to hit the handful of lines in the horner method or early exits other methods, I think they're also being hit from the random generator. But coverage for these is either 96.1% or 100%, with most files at 100% |
Thanks! I believe this looks very good! Do you agree, @fredrik-johansson? |
Yes, looks good. |
In some work on python-flint I noticed that the
fmpz_mod_mpoly_compose_fmpz_mod_mpoly_gens
function was documented, but unimplemented. I've looked through the git history appears it was never implemented, only added as a doc string.This PR adds that function and the other missing
fmpz_mod_mpoly_compose_horner
function. As well as a set of test cases for the compose functions.I've retained the copyright headers from the
fmpz_mpoly
andnmod_mpoly
related files as these new ones are largely a copy-paste + search-replace + minor edits.One test cases is commented out as I'm not sure why it isn't passing. I believe it's near identical to it's counter parts. Something is up with it. I'll marked the PR as draft until I can take a deeper look.
I'm not sure why these functions were not implemented previously. Perhaps just an oversight.