Clean up interface to the PARI library

[pjbruin]

The file sage/libs/pari/gen.pyx is too big and contains too many different things. For clarity and maintainability, it should be split into two parts:

• gen.pyx containing the class gen;
• pari_instance.pyx containing the class PariInstance and some utility functions;
  More cleaning-up will be done in Clean up interface to the PARI library, II #15461.

Depends on #9640
Depends on #10018
Depends on #11868

Component: interfaces

Keywords: pari

Author: Peter Bruin

Branch/Commit: u/jdemeyer/ticket/15185 @ 7ec74e0

Reviewer: Jeroen Demeyer

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

[jdemeyer]

comment:1

The t0GEN stuff is #11868.

[pjbruin]

Branch: u/pbruin/15185

[pjbruin]

Work Issues: wait for #10018

[pjbruin]

Changed dependencies from #9640 to #9640 #10018

[pjbruin]

Commit: 2344f0b

[pjbruin]

Author: Peter Bruin

[pjbruin]

Changed dependencies from #9640 #10018 to #9640, #10018

[jdemeyer]

comment:7

I think this ticket is a typical "patchbomb" which should be avoided. In particular, the replacing of the global variables t0... and the re-organizing of the .py(x) files are both big changes which should be on different tickets. For the former, I already created #11868 for this.

[pjbruin]

comment:8

Replying to @jdemeyer:

I think this ticket is a typical "patchbomb" which should be avoided.

I know. The problem is that when I started this clean-up operation, it wasn't clear what the end result was going to look like. I created this ticket -- and the list of separate issues -- only after reaching a state that worked again, and didn't know about #11868. Cutting it up into separate tickets is easier said than done; it will probably become more manageable by using Git, but it will still take a lot of effort.

[pjbruin]

Changed dependencies from #9640, #10018 to #9640, #10018, #11868

[pjbruin]

Changed work issues from wait for #10018 to rebase

[pjbruin]

comment:9

As a first step I split off the changes eliminating the global GEN variables as a patch on #11868. This will certainly have to be rebased, but that had to be done anyway because of #10018.

[jdemeyer]

comment:10

To ease rebasing and reviewing, I recommend you to make this ticket absolutely minimal (only move stuff, nothing else): everything which can be done on a different ticket, should be done so.

[pjbruin]

comment:11

Replying to @jdemeyer:

To ease rebasing and reviewing, I recommend you to make this ticket absolutely minimal (only move stuff, nothing else): everything which can be done on a different ticket, should be done so.

That is more or less my plan, after #11868 is finished.

[jdemeyer]

comment:13

Are we sure we really want to do this? I'm afraid that performance is going to be worse because Cython has an overhead when calling functions in a different module.

[pjbruin]

comment:14

Replying to @jdemeyer:

Are we sure we really want to do this? I'm afraid that performance is going to be worse because Cython has an overhead when calling functions in a different module.

Is this because the @cython.final decorator on an extension type T currently does not forbid subtyping T in a different Cython module (so that methods can be called directly in the same module, but only via a vtab in a different module)?

If this is what you mean, could the overhead be avoided by making new_gen() and clear_stack() (which will be responsible for most of the cross-module calls) functions instead of methods?

[pjbruin]

Changed branch from u/pbruin/15185 to u/pbruin/15185-split_up_pari_interface

[pjbruin]

Changed work issues from rebase to none

[pjbruin]

Changed commit from 2344f0b to 11a3bfb

[pjbruin]

comment:16

Here is a new branch which moves PariInstance to a separate file and otherwise is close to minimal.

It is true that the Cython-generated code has some overhead in for the cross-module method calls. I think it is very small, but I will try to do some timings. Depending on the outcome, it might be reasonable to duplicate a small amount of code to avoid these cross-module calls. I think it is better to do that as part of #15461, though.

---

New commits:

[11a3bfb](https://github.com/sagemath/sagetrac-mirror/commit/11a3bfb)	more fixes related to relocation of PariInstance
[34013cc](https://github.com/sagemath/sagetrac-mirror/commit/34013cc)	fixes in sage.libs.pari
[db8cd7e](https://github.com/sagemath/sagetrac-mirror/commit/db8cd7e)	top-level changes related to PariInstance
[fcf7972](https://github.com/sagemath/sagetrac-mirror/commit/fcf7972)	adapt miscellaneous files in the Sage library
[5091563](https://github.com/sagemath/sagetrac-mirror/commit/5091563)	fix imports in gen_py.py
[478dd90](https://github.com/sagemath/sagetrac-mirror/commit/478dd90)	move code from gen.pyx to pari_instance.pyx
[08a278f](https://github.com/sagemath/sagetrac-mirror/commit/08a278f)	create pari_instance.pyx; move some documentation and examples there
[561e39f](https://github.com/sagemath/sagetrac-mirror/commit/561e39f)	create pari_instance.pxd

[sagetrac-git]

Changed commit from 11a3bfb to 95a622a

[sagetrac-git]

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

[95a622a](https://github.com/sagemath/sagetrac-mirror/commit/95a622a)

better fix for real_double.pyx

[jdemeyer]

Changed branch from u/pbruin/15185-split_up_pari_interface to u/jdemeyer/ticket/15185

[jdemeyer]

Reviewer: Jeroen Demeyer

[jdemeyer]

Changed commit from 95a622a to 7ec74e0

[jdemeyer]

comment:19

Additional commit needs review. Apart from this, everything looks fine.

---

New commits:

[7ec74e0](https://github.com/sagemath/sagetrac-mirror/commit/7ec74e0)

PARI cleanup: reviewer patch

[pjbruin]

comment:20

I wasn't sure whether allocatemem() should already be removed from the global namespace (it was only deprecated recently), and whether from sage.libs.pari.all import ... is better than specifying the precise module, but I'm definitely OK with these changes.