Allow multiple instances of the PariInstance object

[jdemeyer]

Since the PariInstance object is really just a callable namespace without state, it should be possible to create multiple instances of it.

The rationale for this is that when multiple python modules depend upon CyPari, each one must instantiate its own PariInstance object, or possibly an object inheriting from it.

Since PariInstance initializes the PARI library upon __init__, we must be careful to do this initialization only once.

When PariInstance.__init__() is called a second time with a size argument which is larger than the current size, we should increase (but never decrease) the PARI stack size but otherwise do nothing.

Depends on #21820

CC: @defeo

Component: interfaces

Keywords: atelierpari2017

Author: Luca De Feo, Jeroen Demeyer

Branch/Commit: b70450d

Reviewer: Vincent Delecroix, Luca De Feo

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

[jdemeyer]

Dependencies: #21820

[defeo]

Branch: u/defeo/allow_multiple_instances_of_the_pariinstance_object

[defeo]

Commit: 061467f

[defeo]

New commits:

061467f

Allow the PariInstance object to be allocated multiple times.

[defeo]

Author: Luca De Feo

[defeo]

Changed keywords from none to atelierpari2017

[videlec]

comment:5

I don't understand clearly why we should allow multiple pari instances. Could it be precised in the ticket description? (we discussed it yesterday but I don't quite remember)

[videlec]

comment:6

minor remark: there are trailing whitespaces!

[videlec]

Reviewer: Vincent Delecroix

[sagetrac-git]

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

aa6e5dd

Removed trailing withespaces

[sagetrac-git]

Changed commit from 061467f to aa6e5dd

[defeo]

comment:9

Note that an alternate, reasonable, design would initialize the PARI library at module load, rather than in __init__. However this ticket is not concerned with this.

With the current design, we have the (dubious) advantage of being able to call pari_close() (via PariInstance._close(), and then reinitialize PARI by creating a new PariInstance object at leisure.

[videlec]

comment:11

pari is initialized at module load since a global pari_instance is created! Calling _close is dangerous since this global instance is still accessible... we might want to document this bug-feature

[jdemeyer]

Changed branch from u/defeo/allow_multiple_instances_of_the_pariinstance_object to u/jdemeyer/allow_multiple_instances_of_the_pariinstance_object

[jdemeyer]

Changed commit from aa6e5dd to 9df4382

[jdemeyer]

Changed author from Luca De Feo to Luca De Feo, Jeroen Demeyer

[jdemeyer]

comment:13

Replying to @videlec:

pari is initialized at module load since a global pari_instance is created!

But that will change. cypari2 should not do that.

---

New commits:

9df4382

Move initialization of PARI to PariInstance.__cinit__

[sagetrac-git]

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

c2d09af

Improve documentation of _close()

[sagetrac-git]

Changed commit from 9df4382 to c2d09af

[sagetrac-git]

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

4b81781

Move _close() method right after __init__()

[sagetrac-git]

Changed commit from c2d09af to 4b81781

[jdemeyer]

comment:16

New version, needs review...

[defeo]

Changed branch from u/jdemeyer/allow_multiple_instances_of_the_pariinstance_object to u/defeo/allow_multiple_instances_of_the_pariinstance_object

[defeo]

comment:18

All doctests pass, and I like Jeroens modifications.

---

New commits:

80d4098

Typo

[defeo]

Changed reviewer from Vincent Delecroix to Vincent Delecroix, Luca De Feo

[defeo]

Changed commit from 4b81781 to 80d4098

[videlec]

comment:19

What should I expect from

sage: from sage.libs.cypari2.pari_instance import PariInstance
sage: p = PariInstance()
sage: p._close()
sage: p('12')

[jdemeyer]

comment:20

Replying to @videlec:

What should I expect from

sage: from sage.libs.cypari2.pari_instance import PariInstance
sage: p = PariInstance()
sage: p._close()
sage: p('12')

A segmentation fault I guess.

[videlec]

comment:21

Replying to @jdemeyer:

Replying to @videlec:

What should I expect from

sage: from sage.libs.cypari2.pari_instance import PariInstance
sage: p = PariInstance()
sage: p._close()
sage: p('12')

A segmentation fault I guess.

Shouldn't it be documented as I mentioned in [comment:11]?

[jdemeyer]

comment:22

There is already the comment

            Call this method at you own risk if you really need to free
            the memory used by PARI.

[videlec]

comment:23

Replying to @jdemeyer:

There is already the comment

            Call this method at you own risk if you really need to free
            the memory used by PARI.

This comment is not clear enough. The sentence says that the memory is freed. Not that you will obtain a SEGFAULT if you try to use pari again. Moreover, creating a new pari instance would make pari available again (right?).

[jdemeyer]

Changed branch from u/defeo/allow_multiple_instances_of_the_pariinstance_object to u/jdemeyer/allow_multiple_instances_of_the_pariinstance_object

[videlec]

Changed commit from 80d4098 to b70450d

[videlec]

New commits:

b70450d

Improve documentation of _close()

[vbraun]

Changed branch from u/jdemeyer/allow_multiple_instances_of_the_pariinstance_object to b70450d