#22746 closed enhancement (fixed)

Update sage.lfunctions.dokchitser.Dokchitser to use only one gp interpreter

Reported by: embray Owned by:
Priority: major Milestone: sage-8.0
Component: elliptic curves Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: 2cc45fa (Commits) Commit: 2cc45fa219cfbe5a55ba3722ae329e706f2ca0ee
Dependencies: Stopgaps:

Description

This updates the Dokchitser class to share a single gp interpreter instance for all instances of the Dokchitser class. Previously each instance spun up its own gp interpreter, and would keep it running until the instance is garbage-collected.

That situation was less than ideal in general, and even worse on platforms where file descriptors are limited (which could be any depending on the user's ulimits). See #22718

This provides a temporary workaround to something like #15809, which might be a preferable solution. But this solution was quick to hack together, works now, is needed for Cygwin, and will be easy enough to excise once a better solution is available. Please read the commit message for more implementation details.

(please set the correct component if not "elliptic curves"--it just seemed the most relevant out of what was available)

Change History (11)

comment:1 Changed 18 months ago by embray

A possible enhancement would be to allow an N-process pool, for faster parallel computation. But I don't know if that's really an important use-case here.

comment:2 Changed 18 months ago by embray

  • Status changed from new to needs_review

comment:3 Changed 18 months ago by git

  • Commit changed from 0af0f4c3b78f8fffa69c644e8da1c427e2da8b17 to 2573de5b00d1839ea85cc3ac2bfc5b59c2f843e8

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2573de5Modify the Dokchitser class to only require one gp interpreter which can

comment:4 Changed 17 months ago by git

  • Commit changed from 2573de5b00d1839ea85cc3ac2bfc5b59c2f843e8 to 44fe7702457325c6ec5b6bebbbce0e3a68ec3be0

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

10fc86dThese variables aren't actually globals in this case and don't need to be indexed.
44fe770Unforunate (but working) hack needed in case any of the instantiation 'pre-code' references any of the indexed globals in the script template.

comment:5 Changed 17 months ago by git

  • Commit changed from 44fe7702457325c6ec5b6bebbbce0e3a68ec3be0 to 063041cb44f3ee42626ce6848b705b58a401f497

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

063041cUse the appropriate built-in method for this

comment:6 Changed 17 months ago by embray

  • Status changed from needs_review to needs_work

Still finding new issues with this...

comment:7 Changed 17 months ago by git

  • Commit changed from 063041cb44f3ee42626ce6848b705b58a401f497 to 492f57b5db58a005b7ef5c684edcf90449297be1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

906cc66No need to unset CFLAGS for numpy
2763fc9Deprecation warning message should guide a user to import lcm from sage.arith.all
ee626ea22556: implement periodic points for rational maps
cff3c8922556: fixed doc typo
308d4f222556: fix indeterminacy_locus
4471c21Updated SageMath version to 8.0.beta2
a2a05adModify the Dokchitser class to only require one gp interpreter which can
17b67d4Unforunate (but working) hack needed in case any of the instantiation 'pre-code' references any of the indexed globals in the script template.
7e949baUse the appropriate built-in method for this
492f57bInitializing this attribute should be moved to the end of init_coeffs so that errors in initLdata are still raised as exceptions.

comment:8 Changed 17 months ago by git

  • Commit changed from 492f57b5db58a005b7ef5c684edcf90449297be1 to 2cc45fa219cfbe5a55ba3722ae329e706f2ca0ee

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

dd6c49cModify the Dokchitser class to only require one gp interpreter which can
a16bb52Unforunate (but working) hack needed in case any of the instantiation 'pre-code' references any of the indexed globals in the script template.
1aad963Use the appropriate built-in method for this
2cc45faInitializing this attribute should be moved to the end of init_coeffs so that errors in initLdata are still raised as exceptions.

comment:9 Changed 17 months ago by embray

  • Status changed from needs_work to needs_review

Okay, looks like I have this right now. I actually did have it working before but broke it with an unnecessary earlier commit that stemmed from misunderstanding how variable scoping works in pari.

comment:10 Changed 17 months ago by jpflori

  • Reviewers set to Jean-Pierre Flori
  • Status changed from needs_review to positive_review

comment:11 Changed 17 months ago by vbraun

  • Branch changed from u/embray/lfunctions/dokchitser to 2cc45fa219cfbe5a55ba3722ae329e706f2ca0ee
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.