Opened 5 years ago

Closed 5 years ago

#20094 closed defect (fixed)

Fix and clean up xsrange

Reported by: dkrenn Owned by:
Priority: major Milestone: sage-7.1
Component: misc Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 58d563a (Commits) Commit: 58d563a83521535df25373cbcdadd2d7d9a862c5
Dependencies: #20047 Stopgaps:

Description (last modified by jdemeyer)

We have 

sage: next(xsrange(0, 2^63-2^10+2^9))
...
OverflowError: Python int too large to convert to C long

This is because the internally used variable count is too large. We solve this problem by avoiding xrange().

Besides this, we also move xsrange() and related functions to a new Cython module src/sage/arith/srange.pyx and do some clean-up.

Change History (14)

comment:1 in reply to: ↑ description Changed 5 years ago by dkrenn

Replying to dkrenn:

This is because the internally used variable count is to large.

An easy fix would be to bound this variable count (currently, it seems to be unlikely that more than 2^63-... iterations occur...).

comment:2 Changed 5 years ago by jdemeyer

  • Authors set to Jeroen Demeyer

Alternatively: move xsrange() to misc_c and re-implement it in Cython.

comment:3 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from xsrange fails with large inputs to Fix and clean up xsrange

comment:4 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/xsrange_fails_with_large_inputs

comment:5 Changed 5 years ago by jdemeyer

  • Commit set to 2f7f7582ca3ffcbd74c46dec105aaf57226ee0c7
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

​2f7f758Implement xsrange() in Cython and move to sage.arith.srange

comment:6 Changed 5 years ago by git

  • Commit changed from 2f7f7582ca3ffcbd74c46dec105aaf57226ee0c7 to 808456adebbcece69ed0c382992841ce087eb7bd

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

​808456aFix imports of xsrange and srange

comment:7 Changed 5 years ago by git

  • Commit changed from 808456adebbcece69ed0c382992841ce087eb7bd to b06b3b8f40ebf259de11c7a34b3712fcba978124

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

​b06b3b8Fix imports of xsrange and srange

comment:8 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:9 Changed 5 years ago by git

  • Commit changed from b06b3b8f40ebf259de11c7a34b3712fcba978124 to 58d563a83521535df25373cbcdadd2d7d9a862c5

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

​58d563aImplement xsrange() in Cython and move to sage.arith.srange

comment:10 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

New commits:

​58d563aImplement xsrange() in Cython and move to sage.arith.srange

comment:11 Changed 5 years ago by jdemeyer

  • Dependencies set to #20047

comment:12 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:13 Changed 5 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:14 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/xsrange_fails_with_large_inputs to 58d563a83521535df25373cbcdadd2d7d9a862c5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.