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

support factoring polynomials modulo prime powers #34371

Closed
yyyyx4 opened this issue Aug 15, 2022 · 5 comments
Closed

support factoring polynomials modulo prime powers #34371

yyyyx4 opened this issue Aug 15, 2022 · 5 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Aug 15, 2022

Currently, Polynomial_zmod_flint refuses to factor polynomials modulo composites, which is generally not well-defined. However, we can easily make this work modulo prime powers (where it is well-defined) by reusing the implementation for polynomials over p‑adics. This is almost certainly not the fastest way of doing it, but at least it works.

Component: algebra

Author: Lorenz Panny

Branch/Commit: 19018bb

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/34371

@yyyyx4 yyyyx4 added this to the sage-9.7 milestone Aug 15, 2022
@tscrim
Copy link
Collaborator

tscrim commented Aug 16, 2022

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 16, 2022

comment:2

It probably would be too much trouble to separate out the algorithm into a separate function rather than converting the elements by changing the base ring. If it is actually simple, then please do so. Otherwise, green bot => positive review.

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 16, 2022

comment:3

The implementation for ℤₚ[x] relies on PARI, whereas the implementation of (ℤ/n)[x] is based on FLINT. I suppose we could inline the PARI conversion and call PARI without explicitly changing to ℤₚ, but that's already much more effort than this and I'm not sure how much it would actually help (it saves some Python overhead, but the conversion is still needed).

Personally I'd argue that this is an easy way to make something work that didn't work previously, and we can improve the performance later if it turns out to be a problem.

@tscrim
Copy link
Collaborator

tscrim commented Aug 16, 2022

comment:4

Replying to @yyyyx4:

The implementation for ℤₚ[x] relies on PARI, whereas the implementation of (ℤ/n)[x] is based on FLINT. I suppose we could inline the PARI conversion and call PARI without explicitly changing to ℤₚ, but that's already much more effort than this and I'm not sure how much it would actually help (it saves some Python overhead, but the conversion is still needed).

It also means you get an additional transient object that takes up time and memory, which could matter if the polynomial is "big". However...

Personally I'd argue that this is an easy way to make something work that didn't work previously, and we can improve the performance later if it turns out to be a problem.

I agree fully. Only to be done if it was clearly easy.

@vbraun
Copy link
Member

vbraun commented Aug 30, 2022

Changed branch from public/polynomial_factorization_modulo_prime_powers to 19018bb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants