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

further work needed on fast Sage --> PARI int conversion #875

Closed
williamstein opened this issue Oct 13, 2007 · 3 comments
Closed

further work needed on fast Sage --> PARI int conversion #875

williamstein opened this issue Oct 13, 2007 · 3 comments

Comments

@williamstein
Copy link
Contributor

This is further work needed after #467.

22:42 < cwitty_> I was looking at #467, and I just crashed SAGE with a PARI stack overflow.
22:43 < cwitty_> I thought the stack was supposed to resize automatically, or something?  (Or at least
                 not crash SAGE.)
22:44 < cwitty_> sage: n = 10^10000000
22:44 < cwitty_> sage: %time _ = pari(n)
22:44 < cwitty_>   ***   the PARI stack overflows !
22:44 < cwitty_>   current stack size: 8000000 (7.629 Mbytes)
22:44 < cwitty_>   [hint] you can increase GP stack with allocatemem()
22:44 < cwitty_> /home/cwitty/sage/local/bin/sage-sage: line 205: 25703 Aborted
                 sage-ipython -c "$SAGE_STARTUP_COMMAND;" "$@"
22:44 < cwitty_> (This is after I applied the patch from #467.)
22:45 < williamstein> weird.
22:45 < williamstein> it should automatically double *if* the author correctly uses _sig_on/_sig_off
22:47 < cwitty_> This is the ZZ->Pari fast coercion patch, and I'm pretty sure (from skimming the patch)
                 that he never touches _sig_on/_sig_off.  So that's probably it.

THE SOLUTION

Need to move code for _pari_c to gen.pyx as a method off of the Pari object.
Then wrap the call to the function in convert.c in _sig_on / _sig_off.
The _sig_on / _sig_off macros are specially constructed only in gen.pyx
to automatically double the pari stack if we run out of memory.

ALSO -- I think #467 should be better documented. Craig explained
to me how he is "hacking with the internals" of the python/c api. This should be explained even more in the code.

Component: basic arithmetic

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

@williamstein

This comment has been minimized.

@craigcitro craigcitro self-assigned this Oct 13, 2007
@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title further work needed on frast Sage --> PARI int conversion further work needed on fast Sage --> PARI int conversion Oct 20, 2007
@williamstein williamstein modified the milestones: sage-2.8.8, sage-2.8.9 Oct 21, 2007
@sagetrac-mabshoff sagetrac-mabshoff mannequin modified the milestones: sage-2.8.9, sage-2.8.10 Oct 25, 2007
@craigcitro
Copy link
Member

comment:7

This patch fixes the problem above, and adds a doctest about it. It took a bit of fiddling to get this to work -- the point is that you need to load the Integer type in order to make this work in gen.pyx, but you can't do that at compile time (it would be a circular dependency). This is further compounded by the fact that the code in gen.pyx actually gets used in the process of loading other parts of the Sage library -- that is, you create and use Sage's PariInstance in the process of loading various modules, so things had to be touched up in a few places to get things to load.

Also, re: William's note above about "hacking the Python/C API" -- that's not for this patch, that's for the upcoming patch on ticket 864. That might have to wait until 2.9 at the rate our new versions are coming these days, though. ;)

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Oct 27, 2007

comment:8

Attachment: trac_875.hg.gz

@sagetrac-cwitty sagetrac-cwitty mannequin closed this as completed Oct 27, 2007
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

2 participants