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

update giac spkg #19873

Closed
frederichan-IMJPRG opened this issue Jan 12, 2016 · 46 comments
Closed

update giac spkg #19873

frederichan-IMJPRG opened this issue Jan 12, 2016 · 46 comments

Comments

@frederichan-IMJPRG
Copy link

update the giac spkg to the current upstream stable version: 1.2.2-37

The tarball built from spkg-src to put in upstream/ can be downloaded at

http://webusers.imj-prg.fr/~frederic.han/xcas/sage/giac-1.2.2.37.tar.gz

Update the giacpy spkg to 0.5.5. The tarball to put in upstream can be downloaded at

http://webusers.imj-prg.fr/~frederic.han/xcas/giacpy/sage/giacpy-0.5.5.tar.gz

This new release of giacpy is just an update of giac keywords + an update of setup.py for #20258. cf https://gitlab.math.univ-paris-diderot.fr/han/giacpy-sage

From 1.2.0.x to 1.2.2.x some multithreading was introduced in groebner basis computation over prime fields.

on a Xeon E5520@2.27Ghz (2 cpu with 4 cores) I got a groebner basis (from the sage with conversion to sage and 8threads allowed) for cyclic9 over GF(101)
giac 1.2.0: Time: CPU 1328.82 s, Wall: 1328.61 s
giac 1.2.2: Time: CPU 824.29 s, Wall: 326.89 s

Component: packages: optional

Author: Frederic Han

Branch/Commit: a959e08

Reviewer: Vincent Delecroix

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

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

04b4906Merge branch 'develop' of git://trac.sagemath.org/sage into giac122
1ba0c18update to 1.2.2.21
215ae00update to giac source 1.2.2.23
275aa45update doctest to the timing strings added in gbasis in giac122.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 27, 2016

Changed commit from a13d26c to 275aa45

@frederichan-IMJPRG

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2016

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

276017eMerge tag '7.1.beta0' of git://trac.sagemath.org/sage into giac122
ad95056update to giac upstream source 1.2.2.27

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2016

Changed commit from 275aa45 to ad95056

@frederichan-IMJPRG

This comment has been minimized.

@frederichan-IMJPRG

This comment has been minimized.

@frederichan-IMJPRG
Copy link
Author

comment:9

I have found that eliminate is slower with this version of giac
http://xcas.e.ujf-grenoble.fr/XCAS/viewtopic.php?f=3&t=1694
so I put this in needwork state to wait if this can be easily fixed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2016

Changed commit from ad95056 to df2422e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 7, 2016

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

df2422eupdage giacpy to 0.5.2 (update giac keywords)

@frederichan-IMJPRG

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 15, 2016

Changed commit from df2422e to bb55869

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 15, 2016

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

bb55869update to giac 1.2.2-29

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

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

9325136update to giac 1.2.2.35

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

Changed commit from bb55869 to 9325136

@frederichan-IMJPRG

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

Changed commit from 9325136 to 004ca90

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

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

004ca90giacpy 0.5.3 (use cythonize in setup.py)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

Changed commit from 004ca90 to 0b61e86

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

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

0b61e86run doctests from the pkgs directory

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

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

9e017b7clean up for spkg-check

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 25, 2016

Changed commit from 0b61e86 to 9e017b7

@frederichan-IMJPRG

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Mar 29, 2016

comment:22

I just tried to install the package and ran into

$ sage -p giacpy
Found local metadata for giacpy-0.5.3
Using cached file /opt/sage/upstream/giacpy-0.5.3.tar.gz
giacpy-0.5.3
====================================================
Setting up build directory for giacpy-0.5.3
mv: cannot stat 'giacpy-0.5.3*': No such file or directory
Finished set up
****************************************************

@videlec videlec modified the milestones: sage-7.0, sage-7.2 Mar 29, 2016
@frederichan-IMJPRG
Copy link
Author

comment:23

and the built doesn't start after this message?
with 7.1.beta6 I have also this message but the built start. Is it a problem that the tarball giacpy-0.5.3.tar.gz looks like this:

tar -tzf giacpy-0.5.3.tar.gz 
src/
src/exemple.sage
src/giacpy.pyx
src/misc.h
src/sage-release.txt
src/README.txt
src/INSTALL.txt
src/setup.py
src/giacpy.pxd
src/keywords.pxi

@frederichan-IMJPRG
Copy link
Author

comment:24

with 7.2.beta1 I have doctest failure due to

    doctest:858: DeprecationWarning: when constructing a matrix, the ring must be the first argument
    See http://trac.sagemath.org/20015 for details.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2016

Changed commit from 31f2368 to 3df5520

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2016

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

3187337Merge branch 'develop' of git://trac.sagemath.org/sage into giac122
e6ece40giacpy 0.5.4 (doctest fixes for deprecated syntax: matrix(M,ring)
3df5520Fix doctests for deprecated matrix(M,ring)

@frederichan-IMJPRG

This comment has been minimized.

@videlec
Copy link
Contributor

videlec commented Mar 31, 2016

comment:27

Replying to @frederichan-IMJPRG:

and the built doesn't start after this message?
with 7.1.beta6 I have also this message but the built start. Is it a problem that the tarball giacpy-0.5.3.tar.gz looks like this:

tar -tzf giacpy-0.5.3.tar.gz 
src/
src/exemple.sage
src/giacpy.pyx
src/misc.h
src/sage-release.txt
src/README.txt
src/INSTALL.txt
src/setup.py
src/giacpy.pxd
src/keywords.pxi

It does. The thing is that the sage install script is starting with the command tar xf PKG and then mv PKG_DIR/ src/. It would be better if the archive name would take the same name as the directory. It would be less error-prone.

@videlec
Copy link
Contributor

videlec commented Mar 31, 2016

comment:28

Following the "Short usage" example on your webpage I got two warnings (sage-7.2.beta1):

sage: import giacpy
...: DeprecationWarning: import "cysignals/signals.pxi" instead
of "sage/ext/interrupt.pxi"
See http://trac.sagemath.org/20002 for details.

and

sage: f.diff()
...: DeprecationWarning: DisplayFormatter._ipython_display_formatter_default
is deprecated: use @default decorator instead.
15*(x+y+z+1)^14

@frederichan-IMJPRG
Copy link
Author

comment:29

Replying to @videlec:

Following the "Short usage" example on your webpage I got two warnings (sage-7.2.beta1):

...

sage: f.diff()
...: DeprecationWarning: DisplayFormatter._ipython_display_formatter_default
is deprecated: use @default decorator instead.
15*(x+y+z+1)^14

I have no idea to fix this warning. It looks similar to #20206. NB it looks to appear only once. (ie doing another f.diff() is OK)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2016

Changed commit from 3df5520 to a959e08

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 1, 2016

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

a959e08use cysignals instead of deprecated interrupt.pxi

@frederichan-IMJPRG

This comment has been minimized.

@frederichan-IMJPRG
Copy link
Author

New commits:

a959e08use cysignals instead of deprecated interrupt.pxi

@frederichan-IMJPRG
Copy link
Author

comment:32

NB: the current 1.2.2.37 giac tar ball that I have made with spkg-src also expand as src. So I am waiting for 1.2.2.39 release (parisse said next week) to fix this.

@videlec
Copy link
Contributor

videlec commented Apr 1, 2016

comment:34

The examples from your webpage in the section " Syntaxes with reserved or unknown Python symbols" do not work. There is no assume, cos, sin, solve or fMax imported from giacpy

sage: from giacpy import *
// Giac share root-directory:/opt/sage/local/share/giac/
// Giac share root-directory:/opt/sage/local/share/giac/
Help file /opt/sage/local/share/giac/doc/fr/aide_cas not found
Added 0 synonyms
sage: solve.__module__
'sage.symbolic.relation'
sage: sin.__module__
'sage.functions.trig'
sage: cos.__module__
'sage.functions.trig'
sage: assume.__module__
'sage.symbolic.assumptions'

@frederichan-IMJPRG
Copy link
Author

comment:35

I think that you are looking at the doc of the Python version of giacpy. It differs from the sage version (gmp, avoid conflics with SR...)
Similar doc is available with

libgiac?

@videlec
Copy link
Contributor

videlec commented Apr 1, 2016

comment:36

All right. I was able to run the doctests after decompressing the archive

$ sage -t --long --force-lib exemple.sage giacpy.pyx
Doctesting 2 files.
sage -t --long exemple.sage
    [0 tests, 0.00 s]
sage -t --long giacpy.pyx
    [221 tests, 23.89 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 24.0 seconds
    cpu time: 24.0 seconds
    cumulative wall time: 23.9 seconds

This is good enough for this version...

Though

  1. you should think about fitting Sage conventions for doctests:

    • a = b instead of a=b for affectation

    • no semicolon at the end of the statement

  2. The way the two packages are organized do not follow the Sage conventions, see Packaging from the developer manual. In particular, it would be good to have a test suite for giac. And to explicit the dependencies of the packages.

@videlec
Copy link
Contributor

videlec commented Apr 1, 2016

Reviewer: Vincent Delecroix

@frederichan-IMJPRG
Copy link
Author

comment:37

Replying to @videlec:

All right. I was able to run the doctests after decompressing the archive

$ sage -t --long --force-lib exemple.sage giacpy.pyx
Doctesting 2 files.
sage -t --long exemple.sage
    [0 tests, 0.00 s]
sage -t --long giacpy.pyx
    [221 tests, 23.89 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 24.0 seconds
    cpu time: 24.0 seconds
    cumulative wall time: 23.9 seconds

This is good enough for this version...

Thank you

Though

...

  1. The way the two packages are organized do not follow the Sage conventions, see Packaging from the developer manual. In particular, it would be good to have a test suite for giac. And to explicit the dependencies of the packages.

I don't understand,

env SAGE_CHECK=yes make giac

has a long test suite, and build/pkgs/giac and giacpy have both a dependencies file.

https://github.com/sagemath/sagetrac-mirror/blob/develop/build/pkgs/giac?id=98506d3754ddb14ca4548d644b2ed6b7a2d2a5d7

https://github.com/sagemath/sagetrac-mirror/blob/develop/build/pkgs/giacpy?id=98506d3754ddb14ca4548d644b2ed6b7a2d2a5d7

NB: running the giacpy testsuite is not enough for a review because some sage doctests have the # optional - giac or giacpy flag.

src/sage/interfaces/giac.py
src/sage/matrix/matrix1.pyx 
src/sage/modules/free_module_element.pyx 
src/sage/symbolic/expression.pyx 
src/sage/calculus/calculus.py
src/sage/libs/giac.py
src/sage/rings/polynomial/multi_polynomial_ideal.py

@vbraun
Copy link
Member

vbraun commented Apr 2, 2016

Changed branch from u/frederichan/giac122 to a959e08

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

3 participants