#11563 closed enhancement (fixed)
Make lrcalc a standard package
Reported by:  nthiery  Owned by:  sagecombinat 

Priority:  major  Milestone:  sage5.2 
Component:  combinatorics  Keywords:  lrcalc, symmetric functions, days38 
Cc:  sagecombinat, fbissey, asbuch, saliola  Merged in:  sage5.2.rc0 
Authors:  Nicolas M. Thiéry  Reviewers:  Anne Schilling, Jeroen Demeyer, John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  #10333  Stopgaps: 
Description (last modified by )
#10333 implements an interface to lrcalc, using an optional spkg. According to the vote on http://groups.google.com/group/sagedevel/browse_thread/thread/2e7114375f6f88a5/, there is a consensus on making lrcalc a standard spkg, after a one release probation period in order to follow the official rule.
 spkg: add lrcalc1.1.6beta1.spkg as standard spkg.
 apply trac_11563lrcalc_standard_packagent.patch to the Sage library.
 apply trac_11563lrcalcsagerootnt.patch to
SAGE_ROOT
.
Attachments (4)
Change History (50)
comment:1 Changed 7 years ago by
 Cc fbissey added
comment:2 Changed 6 years ago by
 Description modified (diff)
 Keywords lrcalc symmetric functions added
 Milestone changed from sage5.0 to sage5.1
 Reviewers set to Anne Schilling
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Keywords days38 added
comment:4 Changed 6 years ago by
 Description modified (diff)
comment:5 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:6 Changed 6 years ago by
Thanks Anne!
Same comment as Mike :)
comment:7 followup: ↓ 8 Changed 6 years ago by
 Status changed from positive_review to needs_work
This needs patches to spkg/install
and spkg/standard/deps
.
comment:8 in reply to: ↑ 7 ; followup: ↓ 9 Changed 6 years ago by
Hi Jeroen,
Replying to jdemeyer:
This needs patches to
spkg/install
andspkg/standard/deps
.
Ah sorry, I missed that (that's my first standard spkg). Do you mind reviewing my changes? You probably know better than anyone about them :)
Thanks in advance!
Cheers,
Nicolas
comment:9 in reply to: ↑ 8 Changed 6 years ago by
Replying to nthiery:
Hi Jeroen,
Replying to jdemeyer:
This needs patches to
spkg/install
andspkg/standard/deps
.Ah sorry, I missed that (that's my first standard spkg). Do you mind reviewing my changes? You probably know better than anyone about them :)
Thanks in advance!
Cheers,
Nicolas
What is the status on this? It would be good to get this patch into sage soon so we can build on it (and avoid using symmetrica).
Thanks,
Anne
comment:10 followup: ↓ 12 Changed 6 years ago by
You should use $MAKE
instead of make
in spkginstall
and spkgcheck
.
Some of the files in the spkg are not worldreadable. Please fix this with
chmod ugo+rX R src
The SAGE_ROOT
patch trac_11563lrcalcsagerootnt.patch gets positive_review from me.
Finally: adding a new standard package should always be discussed on the sagedevel mailing list.
comment:11 Changed 6 years ago by
 Description modified (diff)
 Reviewers changed from Anne Schilling to Anne Schilling, Jeroen Demeyer
comment:12 in reply to: ↑ 10 Changed 6 years ago by
 Status changed from needs_work to needs_review
Replying to jdemeyer:
You should use
$MAKE
instead ofmake
inspkginstall
andspkgcheck
.Some of the files in the spkg are not worldreadable. Please fix this with
chmod ugo+rX R src
Thanks for catching those. Done. Anne: may I let you set the final positive review? Here is the diff in the updated version I am about to upload (sorry, it does not show up permission changes):
diff git a/spkgcheck b/spkgcheck  a/spkgcheck +++ b/spkgcheck @@ 8,7 +8,7 @@ fi cd src make check +$MAKE check if [ $? ne 0 ]; then echo "Error testing lrcalc." exit 1 diff git a/spkginstall b/spkginstall  a/spkginstall +++ b/spkginstall @@ 14,13 +14,13 @@ if [ $? ne 0 ]; then exit 1 fi make +$MAKE if [ $? ne 0 ]; then echo "Error building lrcalc." exit 1 fi make install +$MAKE install if [ $? ne 0 ]; then echo "Error installing lrcalc." exit 1
The
SAGE_ROOT
patch trac_11563lrcalcsagerootnt.patch gets positive_review from me.
Thanks.
Finally: adding a new standard package should always be discussed on the sagedevel mailing list.
We did it a long time ago :)
Cheers,
Nicolas
comment:13 followup: ↓ 14 Changed 6 years ago by
 Description modified (diff)
comment:14 in reply to: ↑ 13 Changed 6 years ago by
comment:15 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:16 followup: ↓ 17 Changed 6 years ago by
 Status changed from positive_review to needs_work
When you change a package, you should change the version number (say, to lrcalc1.1.6beta.p0.spkg
) and change SPKG.txt
accordingly.
comment:17 in reply to: ↑ 16 Changed 6 years ago by
Replying to jdemeyer:
When you change a package, you should change the version number (say, to
lrcalc1.1.6beta.p0.spkg
) and changeSPKG.txt
accordingly.
Ok, there it is ...
comment:18 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:19 followup: ↓ 20 Changed 6 years ago by
 Description modified (diff)
I think that in SPKG.txt, at the top it should just say "lrcalc" rather than "lrcalc 1.1.6 beta p0": the version number should just be in the ChangeLog (for ease of updating later). I took the liberty of creating a new spkg (committed in your name, Nicolas). I'm also attaching a patch showing the changes.
comment:20 in reply to: ↑ 19 Changed 6 years ago by
Replying to jhpalmieri:
I think that in SPKG.txt, at the top it should just say "lrcalc" rather than "lrcalc 1.1.6 beta p0": the version number should just be in the ChangeLog (for ease of updating later). I took the liberty of creating a new spkg (committed in your name, Nicolas). I'm also attaching a patch showing the changes.
All good for me. Thanks.
comment:21 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:22 Changed 6 years ago by
 Milestone changed from sage5.1 to sage5.2
comment:23 Changed 6 years ago by
 Description modified (diff)
comment:24 Changed 6 years ago by
 Description modified (diff)
comment:25 followup: ↓ 26 Changed 6 years ago by
 Status changed from positive_review to needs_work
This needs to be rebased to sage5.1.beta6:
patching file doc/en/reference/libs.rst Hunk #1 FAILED at 26. 1 out of 1 hunk FAILED  saving rejects to file doc/en/reference/libs.rst.rej patching file sage/libs/lrcalc/lrcalc.pyx
comment:26 in reply to: ↑ 25 ; followup: ↓ 27 Changed 6 years ago by
Replying to jdemeyer:
This needs to be rebased to sage5.1.beta6:
patching file doc/en/reference/libs.rst Hunk #1 FAILED at 26. 1 out of 1 hunk FAILED  saving rejects to file doc/en/reference/libs.rst.rej patching file sage/libs/lrcalc/lrcalc.pyx
Arr, what a pain; we are being soooo inefficient on this one.
Ok, I am downloading 5.1.rc0, and rebase this patch this afternoon or so. It would be good to have this spkg in 5.1 for Sage Days 40!
Changed 6 years ago by
comment:27 in reply to: ↑ 26 Changed 6 years ago by
 Status changed from needs_work to needs_review
Replying to nthiery:
Arr, what a pain; we are being soooo inefficient on this one.
Ok, I am downloading 5.1.rc0, and rebase this patch this afternoon or so. It would be good to have this spkg in 5.1 for Sage Days 40!
Ok, I just updated the patch, rebased on 5.1.rc0. The only change I had to do was:

doc/en/reference/libs.rst
diff git a/doc/en/reference/libs.rst b/doc/en/reference/libs.rst
a b to be aware of the modules described in 27 27 28 28 sage/libs/flint/fmpz_poly 29 29 sage/libs/libecm 30 sage/libs/lrcalc/lrcalc 30 31 sage/libs/pari/gen 31 32 sage/libs/ntl/all 32 33 sage/libs/mwrank/all
Back to needs review ...
Jeroen: if you feel like it, feel free to do such trivial rebases directly; it probably will take you less time than going through this whole process.
Cheers,
Nicolas
comment:28 Changed 6 years ago by
 Status changed from needs_review to positive_review
Changed 6 years ago by
comment:29 followup: ↓ 30 Changed 6 years ago by
SAGE_ROOT
patch rebased to the upcoming sage5.2.beta0.
comment:30 in reply to: ↑ 29 Changed 6 years ago by
comment:31 Changed 6 years ago by
 Merged in set to sage5.2.beta1
 Resolution set to fixed
 Status changed from positive_review to closed
comment:32 followup: ↓ 33 Changed 6 years ago by
 Merged in sage5.2.beta1 deleted
 Resolution fixed deleted
 Status changed from closed to new
lrcalc
fails on hawk
(OpenSolaris? 06.200932):
buildbot@hawk:~/sage5.2.beta0$ ./sage gdb   Sage Version 5.2.beta0, Release Date: 20120709   Type "notebook()" for the browserbased notebook interface.   Type "help()" for help.   ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** /export/home/buildbot/sage5.2.beta0/local/bin/sageipython GNU gdb 6.8 Copyright (C) 2008 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i386pcsolaris2.11"... warning: Lowest section in /lib/libdl.so.1 is .dynamic at 00000074 Python 2.7.3 (default, Jul 11 2012, 15:59:32) [GCC 4.4.3 20100112 (prerelease)] on sunos5 Type "help", "copyright", "credits" or "license" for more information. warning: Lowest section in /lib/libintl.so.1 is .dynamic at 00000074 warning: Lowest section in /lib/libpthread.so.1 is .dynamic at 00000074 sage: from sage.libs.lrcalc import lrcalc sage: lrcalc.mult([Integer(3),Integer(2),Integer(1)], [Integer(3),Integer(2),Integer(1)], Integer(3),Integer(2)) Program received signal SIGSEGV, Segmentation fault. 0x00000064 in ?? () (gdb) bt #0 0x00000064 in ?? () #1 0xfd699fb1 in hash_insert (h=0xc107940, k=0xc108c58, v=0x1) at ../src/language/hash.c:52 #2 0xf6504bac in fusion_reduce (lc=0xc107940, rows=3, cols=2, opt_zero=0) at symfcn.c:866 #3 0xf6529a7f in __pyx_pf_4sage_4libs_6lrcalc_6lrcalc_4mult (__pyx_self=0x0, __pyx_args=0x8102374, __pyx_kwds=0x0) at sage/libs/lrcalc/lrcalc.c:2370 #4 0xfee7d7a0 in PyCFunction_Call (func=0xc0de02c, arg=0x8102374, kw=0xc108c58) at Objects/methodobject.c:85 #5 0xfeed917b in PyEval_EvalFrameEx (f=0xc1372ec, throwflag=0) at Python/ceval.c:4021 #6 0xfeed9c8b in PyEval_EvalCodeEx (co=0xc0da020, globals=0x851eb54, locals=0x851eb54, args=0x0, argcount=0, kws=0x0, kwcount=0, defs=0x0, defcount=0, closure=0x0) at Python/ceval.c:3253 [...]
comment:33 in reply to: ↑ 32 Changed 6 years ago by
Could Nicolas and I get an account on hawk to investigate this? Or will you?
Thanks,
Anne
comment:34 Changed 6 years ago by
 Description modified (diff)
comment:35 Changed 6 years ago by
 Description modified (diff)
 Status changed from new to needs_review
Thank to John, I could login on hawk and investigate. It turned out to be a Clevel name conflict with a function called hash_insert
. I used a macro to rename that function in the lrcalc sources.
I also used the occasion to add examples of fusion and quantum calculations in the README and testsuite.
I bumped lrcalc's version accordingly. Anders: please finish up version 1.1.6 from there!
The updated spkg also only installs the lrcalc libraries and headers, not the binaries. Thanks John for pointing this out!
See the attached diff for the changes.
Quick review would be most appreciated to get this in 5.2!!!
Thanks,
Nicolas
comment:36 Changed 6 years ago by
 Cc asbuch saliola added
comment:37 Changed 6 years ago by
 Description modified (diff)
comment:38 Changed 6 years ago by
I should mention: installation and tests passed smoothly on hawk and a ubuntu machine.
comment:39 followup: ↓ 40 Changed 6 years ago by
Since the binaries are no longer installed, the test suite for the package fails (that is, running sage i c lrcalc...
):
Successfully installed lrcalc1.1.6beta1 Running the test suite for lrcalc1.1.6beta1... Making check in mathlib make[1]: warning: jN forced in submake: disabling jobserver mode. make[1]: Nothing to be done for `check'. Making check in lrcoef make[1]: warning: jN forced in submake: disabling jobserver mode. make[1]: Nothing to be done for `check'. Making check in schubert make[1]: warning: jN forced in submake: disabling jobserver mode. make[1]: Nothing to be done for `check'. make[1]: warning: jN forced in submake: disabling jobserver mode. make j2 checkTESTS make[2]: warning: jN forced in submake: disabling jobserver mode. n testing lrcoef/lrcoef 3 2 1  2 1  2 1 ... ok n testing lrcoef/skew 3 2 1  2 1 ... ok n testing lrcoef/skew r 2 3 2 1 / 2 1 ... ok n testing lrcoef/mult 2 1  2 1 ... ok ./testsuite: line 7: mult: command not found n testing mult f 3,2 3 2 1  3 2 1 ... failed: Expected: 1 (4, 4, 4) 1 (5, 4, 3) Got: ./testsuite: line 7: mult: command not found n testing mult q 3,2 3 2 1  3 2 1 ... failed: Expected: 1 (2) 1 (1, 1) Got: n testing lrcoef/coprod 3 2 1 ... ok n testing schubert/schubmult 1 3 2  1 3 2 ... ok FAIL: testsuite ======================================================= 1 of 1 test failed Please report to asbuch at math dot rutgers period edu =======================================================
Otherwise, all tests passed for me on hawk also, and on OS X 10.7. I think that since this had a positive review before, if you can fix spkgcheck so selftests pass, then it should get a positive review.
Changed 6 years ago by
comment:40 in reply to: ↑ 39 Changed 6 years ago by
Replying to jhpalmieri:
Since the binaries are no longer installed, the test suite for the package fails
Oops, fixed (I was calling mult incorrectly in the new tests). Thanks for catching this! Updated spkg and diff attached.
Thanks for the super quick review :)
Cheers,
Nicolas
comment:41 followup: ↓ 42 Changed 6 years ago by
 Status changed from needs_review to positive_review
Looks good now. Passes its test suite and lrcalc.pyx passes doctests on sage.math, hawk, and OS X 10.7.
comment:42 in reply to: ↑ 41 Changed 6 years ago by
Replying to jhpalmieri:
Looks good now. Passes its test suite and lrcalc.pyx passes doctests on sage.math, hawk, and OS X 10.7.
Thanks, John, for running the final tests and giving access to hawk! You should add yourself to the list of reviewers!
comment:43 Changed 6 years ago by
 Reviewers changed from Anne Schilling, Jeroen Demeyer to Anne Schilling, Jeroen Demeyer, John Palmieri
comment:44 Changed 6 years ago by
 Merged in set to sage5.2.rc0
 Resolution set to fixed
 Status changed from positive_review to closed
comment:45 Changed 6 years ago by
Is there some prevision of an official release of lrcalc 1.1.6? I would like to package it in fedora, and believe the sagemath spkg is not mean't to be the upstream tarball. It should be good enough if upstream can make the 1.1.6beta tarball available. But it has plenty of time as I deferred my fedora sagemath package to after fedora 18.
comment:46 Changed 4 years ago by
Please review the trivial followup ticket #16756.
I ran the tests and looked at the documentation. Everything looks fine.
Anne