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

Let exponentiation of polynomial using FLINT nmod type be interrupted #17470

Closed
jpflori opened this issue Dec 8, 2014 · 25 comments
Closed

Let exponentiation of polynomial using FLINT nmod type be interrupted #17470

jpflori opened this issue Dec 8, 2014 · 25 comments

Comments

@jpflori
Copy link
Contributor

jpflori commented Dec 8, 2014

sage: p = next_prime(2**30)
sage: k = GF(p)
sage: x = polygen(k)
sage: x**p
CTRL+C....

CC: @vbraun @jdemeyer

Component: finite rings

Author: Jean-Pierre Flori

Branch/Commit: 0706674

Reviewer: Jeroen Demeyer

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

@jpflori jpflori added this to the sage-6.5 milestone Dec 8, 2014
@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

Commit: ef28e27

@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

Author: Jean-Pierre Flori

@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

Branch: u/jpflori/ticket/17470

@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

New commits:

0aed331Make modular exponentiation of polynomial using FLINT interruptable.
ef28e27Better fix for exponentiation of polys using FLINT nmod and test.

@jpflori

This comment has been minimized.

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2014

comment:2

cdefs.pxi should be interrupt.pxi

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2014

comment:3

I know it's not the topic of this ticket, but why is this code not implemented as direct calls to either nmod_poly_pow() or nmod_poly_powmod_mpz_binexp()?

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2014

comment:4

Also, celement_pow doesn't seem to be documented anywhere...

I do give positive_review to the addition of sig_on() and sig_off() and the doctest changes.

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2014

comment:5

Actually,

sage: alarm(1)
sage: x^n

should be

sage: alarm(1); x^n

@jdemeyer
Copy link
Contributor

jdemeyer commented Dec 9, 2014

comment:6

I created #17476 as possible "meta-ticket".

@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

comment:7

Replying to @jdemeyer:

I know it's not the topic of this ticket, but why is this code not implemented as direct calls to either nmod_poly_pow() or nmod_poly_powmod_mpz_binexp()?

Maybe because the file was written 6 years ago on top of FLINT 1.x :)

@jpflori
Copy link
Contributor Author

jpflori commented Dec 9, 2014

comment:8

As far as old stuff which needs update is concerned, we should also rewrite the NTL wrapping code now that Cython understands C++.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 9, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

12539c9Cleanup exponentiation for polynomial over finite fields using FLINT.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 9, 2014

Changed commit from ef28e27 to 12539c9

@jdemeyer
Copy link
Contributor

comment:11

I get

sage -t --long src/sage/libs/flint/nmod_poly_linkage.pxi
**********************************************************************
File "src/sage/libs/flint/nmod_poly_linkage.pxi", line 498, in sage.libs.flint.nmod_poly_linkage.celement_pow
Failed example:
    alarm(1); x^n
Expected:
    Traceback (most recent call last):
    ...
    AlarmInterrupt
Got:
    Exception (FLINT memory_manager). Unable to allocate memory.
    Traceback (most recent call last):
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/src/sage-config/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 851, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.libs.flint.nmod_poly_linkage.celement_pow[20]>", line 1, in <module>
        alarm(Integer(1)); x**n
      File "sage/rings/polynomial/polynomial_template.pxi", line 625, in sage.rings.polynomial.polynomial_zmod_flint.Polynomial_template.__pow__ (build/cythonized/sage/rings/polynomial/polynomial_zmod_flint.cpp:10790)
      File "sage/libs/flint/nmod_poly_linkage.pxi", line 508, in sage.rings.polynomial.polynomial_zmod_flint.celement_pow (build/cythonized/sage/rings/polynomial/polynomial_zmod_flint.cpp:4815)
      File "sage/ext/c_lib.pyx", line 85, in sage.ext.c_lib.sig_raise_exception (build/cythonized/sage/ext/c_lib.c:971)
    RuntimeError: Aborted
**********************************************************************

@jpflori
Copy link
Contributor Author

jpflori commented Dec 16, 2014

comment:12

Indeed...
Actually I think it will be quite hard to find a test that won't spuriously cause memory errors.

Exponentiation without modular reduction will be either too fast or eat up all the memory.
And modular exponentiation is just fast as well and we can only use longs as exponents right now.

@jdemeyer
Copy link
Contributor

comment:13

Proposal: choose n such that the test passes with 2GB of virtual memory (use ulimit -v 2000000 to test this). If needed, make the alarm time shorter.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 17, 2014

Changed commit from 12539c9 to c781c24

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 17, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

c781c24Modify FLINT exponentiation test to pass with <= 2Gb of virtual mem.

@jdemeyer
Copy link
Contributor

Changed branch from u/jpflori/ticket/17470 to u/jdemeyer/ticket/17470

@jdemeyer
Copy link
Contributor

Changed commit from c781c24 to 0706674

@jdemeyer
Copy link
Contributor

comment:17

I modified the test slightly. Other than that, the patch looks good.


New commits:

0706674Modify doctest

@jpflori
Copy link
Contributor Author

jpflori commented Dec 19, 2014

Reviewer: Jeroen Demeyer

@jpflori
Copy link
Contributor Author

jpflori commented Dec 19, 2014

comment:18

I assume this means positive review...

@vbraun
Copy link
Member

vbraun commented Dec 21, 2014

Changed branch from u/jdemeyer/ticket/17470 to 0706674

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