Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify cythonization of many sage extensions. #15410

Closed
robertwb opened this issue Nov 13, 2013 · 32 comments
Closed

Simplify cythonization of many sage extensions. #15410

robertwb opened this issue Nov 13, 2013 · 32 comments

Comments

@robertwb
Copy link
Contributor

No need to enumerate modules if we don't use special flags.

Component: build

Author: Robert Bradshaw, Jeroen Demeyer

Branch/Commit: 0681b66

Reviewer: Nathann Cohen, Jean-Pierre Flori

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

@robertwb robertwb added this to the sage-5.13 milestone Nov 13, 2013
@robertwb
Copy link
Contributor Author

Branch: u/robertwb/ticket/15410

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

Commit: 94f5a9f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

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

[94f5a9f](https://github.com/sagemath/sagetrac-mirror/commit/94f5a9f)Simplify cythonization of sage/structure

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

Changed commit from 94f5a9f to 5219984

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

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

[5219984](https://github.com/sagemath/sagetrac-mirror/commit/5219984)Simplify category, coding, and ext cythonization.
[708ee36](https://github.com/sagemath/sagetrac-mirror/commit/708ee36)Simplify geometry, games, functions, crypto cythonization.
[d5bb198](https://github.com/sagemath/sagetrac-mirror/commit/d5bb198)Simplify graphs, groups cythonization.
[ee6bff2](https://github.com/sagemath/sagetrac-mirror/commit/ee6bff2)Simplify cythonization for media, matroids, and misc.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

Changed commit from 5219984 to 7caecd0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

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

[7caecd0](https://github.com/sagemath/sagetrac-mirror/commit/7caecd0)Trivial cythonization simplification.

@robertwb robertwb changed the title Simplify cythonization of sage/structure. Simplify cythonization of many sage extensions. Nov 13, 2013
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Nov 13, 2013

comment:7

Hmmmm... Looks like the 'gmp' dependency in sage.quadratic_forms.count_local_2 was useless ?

Nathann

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

Changed commit from 7caecd0 to 7703d9e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 13, 2013

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

[7703d9e](https://github.com/sagemath/sagetrac-mirror/commit/7703d9e)Add missing gmp library dependency.

@robertwb
Copy link
Contributor Author

comment:9

Eventually this will be inferred from the cimports (e.g. of integer_ring). There's a mess of includes and pxi files that needs to be sorted out before that can happen...

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Nov 14, 2013

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Nov 14, 2013

comment:10

Hmmmmmm... Well I applied your three patches, deleted the build/ directory, built Sage again and passes all long tests. I got the following errors, and I believe that none of them comes from your commits

----------------------------------------------------------------------
sage -t --long misc/trace.py  # 2 doctests failed
sage -t --long misc/interpreter.py  # 1 doctest failed
sage -t --long combinat/sf/sfa.py  # 1 doctest failed
sage -t --long calculus/desolvers.py  # 8 doctests failed
sage -t --long libs/symmetrica/symmetrica.pxi  # 1 doctest failed
sage -t --long dev/sagedev.py  # 5 doctests failed
sage -t --long dev/patch.py  # 6 doctests failed
sage -t --long tests/cmdline.py  # 14 doctests failed
sage -t --long tests/interrupt.pyx  # Time out
sage -t --long /home/ncohen/.Sage/src/doc/en/constructions/calculus.rst  # 4 doctests failed
sage -t --long /home/ncohen/.Sage/src/doc/en/prep/Quickstarts/Differential-Equations.rst  # 2 doctests failed
----------------------------------------------------------------------
Total time for all tests: 7355.7 seconds
    cpu time: 4916.7 seconds
    cumulative wall time: 5432.8 seconds

Soooooooo well. I'd say "good to go" :-)

Nathann

@jdemeyer
Copy link

Author: Robert Bradshaw

@jdemeyer jdemeyer modified the milestones: sage-5.13, sage-6.0 Nov 14, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.0, sage-6.1 Dec 17, 2013
@vbraun
Copy link
Member

vbraun commented Dec 20, 2013

comment:13

I'm having problems with this ticket, it works on an incremental build but not in a full rebuild. Dies in conway polynomials: http://build.sagemath.org/sage/builders/%20%20fast%20Volker%20Desktop%20%28Fedora%2019%20x86_64%29%20full/builds/5/steps/compile_1/logs/conway

@sagetrac-felixs
Copy link
Mannequin

sagetrac-felixs mannequin commented Jan 8, 2014

comment:14

while there is no need to enumerate files, the proposed wildcarding has been flagged as bad practice outside the sage/distutils world. see http://www.gnu.org/software/automake/manual/html_node/Wildcards.html for an explanation. or just view the explicit listing of files as an extra integrity check similar to docchecks. you get the idea...

python distutils requires a lot of overhead in file lists, which is bad. but wildcards are worse.

this ticket does not improve anything. better leave it like that (YMMV).

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin added this to the sage-6.3 milestone May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer

This comment has been minimized.

@nexttime nexttime mannequin added t: enhancement and removed t: bug labels Apr 13, 2015
@nexttime nexttime mannequin modified the milestones: sage-6.4, sage-6.6 Apr 13, 2015
@jdemeyer
Copy link

Changed author from Robert Bradshaw to Robert Bradshaw, Jeroen Demeyer

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:26

I'm rebooting this ticket to handle only the trivial cases (where no additional libraries or compiler flags need to be added).

@jdemeyer jdemeyer modified the milestones: sage-6.6, sage-6.8 Jun 24, 2015
@jdemeyer
Copy link

Changed branch from u/robertwb/ticket/15410 to u/jdemeyer/ticket/15410

@jdemeyer
Copy link

New commits:

0681b66Simplify cythonization of many Cython extensions

@jdemeyer
Copy link

Changed commit from 7703d9e to 0681b66

@jpflori
Copy link

jpflori commented Jul 1, 2015

Changed reviewer from Nathann Cohen to Nathann Cohen, Jean-Pierre Flori

@jdemeyer
Copy link

jdemeyer commented Jul 1, 2015

comment:30

See #15412 for a very similar ticket.

@vbraun
Copy link
Member

vbraun commented Jul 2, 2015

Changed branch from u/jdemeyer/ticket/15410 to 0681b66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants