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

Improvements to squarefree_decomposition() for finite fields. #35334

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

tscrim
Copy link
Collaborator

@tscrim tscrim commented Mar 23, 2023

📚 Description

We make some optimizations to the computation of squarefree_decomposition() for polynomials over finite fields. This is a followup to #35323.

We make improvements by only getting the parent of T0 once and manually keep track of its degree.

The current branch

sage: x = GF(4)["x"].gen()
sage: p = x^2 + 1
sage: %timeit p.squarefree_decomposition()
140 µs ± 22.1 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

versus with #35323:

168 µs ± 5.28 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

versus 10.0.beta5:

158 µs ± 6.74 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

📝 Checklist

  • I have made sure that the title is self-explanatory and the description concisely explains the PR.
  • I have linked an issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 23, 2023

It's amazing what a little bit of Cython and avoiding unnecessary calls can do. :)

@remyoudompheng
Copy link
Contributor

Hello, the code change looks OK. By the way I am finalizing a patch to link with NTL for factor and squarefree_decomposition so that only sparse polynomials (anything else?) will rely on the Python implementation.

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 23, 2023

Feel free to put me as a reviewer once that PR is ready.

@github-actions
Copy link

Documentation preview for this PR is ready! 🎉
Built with commit: fdc51c2

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 24, 2023

All tests pass.

@remyoudompheng Did you intend to approve this PR or should this wait for your upcoming change?

@remyoudompheng
Copy link
Contributor

LGTM. The upcoming change is independent.

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 24, 2023

Thank you. Can you also approve the PR? In principle I can do it, but it feels a bit weird.

@tscrim
Copy link
Collaborator Author

tscrim commented Mar 24, 2023

Thanks!

@vbraun vbraun merged commit de75f8f into sagemath:develop Apr 6, 2023
@mkoeppe mkoeppe added this to the sage-10.0 milestone Apr 7, 2023
@tscrim tscrim deleted the finite_fields/improve_squarefree branch May 23, 2023 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants