Skip to content
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 compose_mod and powmod with large exp #174

Merged

Conversation

GiacomoPope
Copy link
Contributor

this PR adds two features which i need for a current project:

  1. optimised polynomial composition when you want to reduce the result modulo another polynomial
  2. f^e mod h when e is very large.

I'm a little rusty as I haven't thought about flint for a while, and there's one TODO for the __pow__ test as now fmpz_mod_poly does not raise an assertion error when a modulus is given generically

cdef nmod_poly res

if e < 0:
raise ValueError("Exponent must be non-negative")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to use inverse_mod here?

Or maybe the caller should just do that if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good question. I don't think we invert where e is negative elsewhere, but this is a reasonable thing to do.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inverting does bring additional complications though because I think then we need the modulus to be irreducible.

@GiacomoPope
Copy link
Contributor Author

Hopefully addressed the above comments

@oscarbenjamin
Copy link
Collaborator

Yep, looks good. Thanks

@oscarbenjamin oscarbenjamin merged commit 069d24d into flintlib:master Aug 6, 2024
30 checks passed
@GiacomoPope GiacomoPope deleted the improve_powmod_and_compose branch August 12, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants