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

Avoid nfinit while factoring polynomials #10910

Closed
lftabera opened this issue Mar 10, 2011 · 21 comments
Closed

Avoid nfinit while factoring polynomials #10910

lftabera opened this issue Mar 10, 2011 · 21 comments

Comments

@lftabera
Copy link
Contributor

In previous versions of pari the options to factor a univariate polynomial over a number field were Trager's or Van Hoeij modular algorithm. The second method is the preferred one, but it used to need a nf structure.

Hence Sage computed nfinit on the number field before factoring the polynomial via Pari.

With current pari version the whole nf structure is not needed. So, the factor routines should not call nfinit that can be a very expensive operation for large fields.

The patch modifies the factor method. If the nf structure is already computed we use it, as it will be faster. If the nf structure is not already computed then do not compute it to factor the polynomial.

See also #11890 for a slightly different solution to the same issue.

Depends on #11130

Upstream: Fixed upstream, in a later stable release.

Component: number fields

Keywords: factorization, pari, nfinit, sd32

Reviewer: Luis Felipe Tabera Alonso

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

@lftabera lftabera added this to the sage-4.7.2 milestone Mar 10, 2011
@lftabera
Copy link
Contributor Author

Author: Luis Felipe Tabera Alonso

@lftabera
Copy link
Contributor Author

comment:2

The doctest failure is known bug that is corrected in #2329.

Depends: #2329

@sagetrac-mariah

This comment has been minimized.

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented May 17, 2011

comment:3

Minor edit of grammer.

@sagetrac-mariah
Copy link
Mannequin

sagetrac-mariah mannequin commented May 18, 2011

comment:4

The patch applied to sage-4.7.rc2 causes the following test to fail:

% ./sage -t  -long -force_lib "devel/sage/sage/rings/polynomial/polynomial_element.pyx"
sage -t -long -force_lib "devel/sage/sage/rings/polynomial/polynomial_element.pyx"
**********************************************************************
File "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-nehalem-fc-review-10910/devel/sage/sage/rings/polynomial/polynomial_element.pyx", line 2704:
    sage: hasattr(N, '_NumberField_generic__pari_nf')
Expected:
    False
Got:
    True
**********************************************************************
1 items had failures:
   1 of 143 in __main__.example_51
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/mariah/.sage//tmp/.doctest_polynomial_element.py
         [23.1 s]
 
----------------------------------------------------------------------
The following tests failed:


        sage -t -long -force_lib "devel/sage/sage/rings/polynomial/polynomial_element.pyx"
Total time for all tests: 23.1 seconds

@lftabera
Copy link
Contributor Author

lftabera commented Jun 7, 2011

comment:5

Just for the record, my patch introduces the following problem, so I need to add a doctest to it since it is an unapparent side effect.

sage: N = NumberField(x^3-2*x+3,'a')
sage: f = N[x](x^2-41943301)
sage: M = N.extension(f,'b')
sage: M.hom([M.gen()])
...
ValueError: no way to map base field to codomain.

@lftabera
Copy link
Contributor Author

lftabera commented Jun 9, 2011

comment:6

I have tracked the latest problem and it is in fact a bug in pari. See upstream report
#1207.

I have corrected some errors in the code and added another doctest.

I am unable to reproduce the doctest failing that Maria points out on sage-4.7

Note: doctest with this patch will fail until the pari bug is solved.

@lftabera
Copy link
Contributor Author

lftabera commented Jun 9, 2011

Upstream: Reported upstream. Little or no feedback.

@lftabera
Copy link
Contributor Author

lftabera commented Jun 9, 2011

Work Issues: pari bug #1207

@lftabera
Copy link
Contributor Author

Changed upstream from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.

@lftabera
Copy link
Contributor Author

comment:8

The problem with pari is solved in the last stable release.

This ticket now depends on #11130 until pari is not updated the patch is not safe.

@lftabera
Copy link
Contributor Author

Dependencies: #11130

@lftabera
Copy link
Contributor Author

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

@lftabera
Copy link
Contributor Author

Attachment: factor_nfinit_free.patch.gz

the good one

@lftabera
Copy link
Contributor Author

comment:9

Rebase against 4.7.1

@williamstein
Copy link
Contributor

Changed keywords from factorization, pari, nfinit to factorization, pari, nfinit, sd32

@jdemeyer

This comment has been minimized.

@lftabera
Copy link
Contributor Author

comment:12

This ticked should be closed as a duplicate of #11890

@jdemeyer
Copy link

Changed author from Luis Felipe Tabera Alonso to none

@jdemeyer
Copy link

Reviewer: Luis Felipe Tabera Alonso

@jdemeyer
Copy link

Changed work issues from pari bug #1207 to none

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