Opened 5 years ago

Closed 5 years ago

#17854 closed task (fixed)

Metaticket: remove c_lib

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.8
Component: c_lib Keywords:
Cc: fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 5ff2d6d (Commits) Commit: 5ff2d6d09345d40ee0cf135da03fed12d588fecb
Dependencies: #18519, #18367, #18450 Stopgaps:

Description (last modified by jdemeyer)

Move c_lib code to Cython modules, or refactor code to no longer use c_lib:

  • #17180: Move and fix rational reconstruction
  • #10103: Remove gmp_globals and gmp.pxi
  • #17668: Replace PY_NEW and PY_NEW_SAME_TYPE by new() method
  • #17726: Replace PY_TYPE() by type()
  • #17725: Replace PY_TYPE_CHECK_EXACT
  • #17800: Replace PY_TYPE_CHECK and IS_INSTANCE by isinstance
  • #17862: Remove use of PY_IS_NUMERIC
  • #17625: Remove init_csage_module()
  • #17789: Remove c_lib/src/ZZ_pylong.cpp
  • #17853: Implement mpz <-> Python int/long in Cython
  • #17788: PARI: store GEN as mpz/mpq
  • #17784: Modernize NTL error handler
  • #17819: Use unsigned long for Integer.divisors
  • #17881: Move memory functions from c_lib to Cython
  • #17882: Cython clean-up in fast_eval.pyx
  • #17889: Unify base_extend, base_extend_c, base_extend_c_impl
  • #17890: Remove _(rich)cmp_c_impl
  • #17900: Setup interrupts in Cython
  • #17915: Stop including mpz_pylong.h and mpz_longlong.h
  • #17916: Move PARI array element assignment macros to parisage.h
  • #17952: Actually remove c_lib files
  • #18007: Move includes of interrupt.pxi to pyx files
  • #18030: Clean-up stdsage.pxi includes
  • #18027: Move interrupts to Cython
  • #18367: Move ntl_wrap to Sage library

Change History (32)

comment:1 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 5 years ago by fbissey

  • Cc fbissey added

That's ambitious and will remove one barrier to having simultaneous python2.7 and python3.x installs at the same time (the others being polybori and pynac).

comment:4 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:8 follow-up: Changed 5 years ago by fbissey

Hum... That will get rid of the last doctest that give a different result with gmp than with mpir.

comment:9 in reply to: ↑ 8 Changed 5 years ago by jdemeyer

Replying to fbissey:

Hum... That will get rid of the last doctest that give a different result with gmp than with mpir.

Which doctest?

comment:10 Changed 5 years ago by fbissey

I should have commented in #17881, commenting here was a mistake. I am talking about the doctest you removed so far in sage/rings/integer.pyx it gives a sligthly different answer for gmp than mpir. This is something we have identified a while ago with Jean-Pierre Flori.

For info this is what it gives for gmp:

**********************************************************************
File "/usr/share/sage/src/sage/rings/integer.pyx", line 1938, in sage.rings.integer.Integer.__pow__
Failed example:
    2^(2^63-2)
Expected:
    Traceback (most recent call last):
    ...
    MemoryError: failed to allocate 1152921504606847008 bytes  
Got:
    gmp: overflow in mpz type
    Traceback (most recent call last):
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 488, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 850, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.rings.integer.Integer.__pow__[27]>", line 1, in <module>
        Integer(2)**(Integer(2)**Integer(63)-Integer(2))
      File "sage/rings/integer.pyx", line 1977, in sage.rings.integer.Integer.__pow__ (build/cythonized/sage/rings/integer.c:12942)
        sig_on()
      File "sage/ext/c_lib.pyx", line 85, in sage.ext.c_lib.sig_raise_exception (build/cythonized/sage/ext/c_lib.c:1035)
        raise RuntimeError(msg)
    RuntimeError: Aborted

comment:11 Changed 5 years ago by jdemeyer

Yes, GMP has some extra error checking that MPIR doesn't have.

Anyway, this doctest will not be removed, just moved to a different file in #17881.

comment:12 Changed 5 years ago by fbissey

Indeed it escaped my attention. So it will just move a bit. No biggy in any case.

comment:13 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:14 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:15 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:17 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:19 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:20 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:21 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:22 Changed 5 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-6.6 to sage-6.7

comment:23 Changed 5 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Dependencies set to #18367
  • Milestone changed from sage-6.7 to sage-6.8

comment:24 Changed 5 years ago by jdemeyer

  • Dependencies changed from #18367 to #18367, #18450

comment:25 Changed 5 years ago by jdemeyer

  • Dependencies changed from #18367, #18450 to #18519, #18367, #18450

comment:26 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17854

comment:27 Changed 5 years ago by jdemeyer

  • Commit set to f35cb98bfe7ff10f48c1d5a0a4fff05d6c2ea74c
  • Status changed from new to needs_review

Last 10 new commits:

57e9278More removals of cdefs.pxi
370106dEven more removals of cdefs
1accad6Remove cdefs.pxi from .pxd/.pxi files
ec497b4Merge tag '6.8.beta0' into HEAD
b5a143fMove ntl_wrap to Sage library
ca87ba3Add Cython patches to improve handling of dependencies
a1ad51cDefine some library dependencies in .pxd/.pxi files
941fa74Merge tag '6.8.beta0' into t/18450/define_library_dependencies_in__pxd_files
f67c28fMerge commit '941fa7420f1942114f88bb3a03bd84bb1997d896' into ticket/17854
f35cb98Remove c_lib

comment:28 Changed 5 years ago by git

  • Commit changed from f35cb98bfe7ff10f48c1d5a0a4fff05d6c2ea74c to 30324559c67836f57637a37d74db40bb05c27660

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

3032455Remove c_lib

comment:29 Changed 5 years ago by git

  • Commit changed from 30324559c67836f57637a37d74db40bb05c27660 to 5ff2d6d09345d40ee0cf135da03fed12d588fecb

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

ae16699Merge tag '6.8.beta2' into HEAD
8b83481Move pari/decl.pxi to pari/paridecl.pxd
1c72dd6partitions_c.cc needs gmp and mpfr libraries
87be62bCorrect Jonathan Bober's name
0be796eAdd ecm library before gmp
d4839b3Merge commit '0be796e608b4e876e01d2e17a30fc5a00867e795' into t/17854/ticket/17854
6cf2f45Merge tag '6.8.beta1' into t/18367/move_ntl_wrap_to_sage_library
97215aeRename ntl_wrap -> ntlwrap
0bd4c02linbox needs ntl
5ff2d6dMerge commit '0bd4c02e112fb242060fd112e7f36a17c36a80e8' into t/17854/ticket/17854

comment:30 Changed 5 years ago by jdemeyer

Merged latest versions of dependencies, now that they have positive_review.

comment:31 Changed 5 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

OK I was a tiny bit too late with my comment in #18367. Anyway, it is now definitely all good to go. I want to see that stuff in 6.8.beta3, it is ready for the bots.

comment:32 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/17854 to 5ff2d6d09345d40ee0cf135da03fed12d588fecb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.