Opened 11 years ago

Last modified 11 years ago

#11321 closed defect

Make lcalc compatible with the new PARI — at Version 14

Reported by: jdemeyer Owned by: tbd
Priority: major Milestone: sage-4.8
Component: packages: standard Keywords: lcalc spkg
Cc: fbissey, rishi Merged in:
Authors: Jeroen Demeyer Reviewers: Volker Braun, Leif Leonhardy
Report Upstream: Reported upstream. Developers acknowledge bug. Work issues:
Branch: Commit:
Dependencies: #11130 Stopgaps:

Status badges

Description (last modified by jdemeyer)

PARI version 2.4.4 (see #11130, now aiming at 2.5.0) no longer has an allocatemoremem() function, which lcalc uses. There is a function allocatemem() which can be used instead.

New spkg: http://boxen.math.washington.edu/home/jdemeyer/spkg/lcalc-1.23.p8.spkg (this will only build after installing the PARI spkg from #11130).

Change History (14)

comment:1 follow-up: Changed 11 years ago by vbraun

  • Reviewers set to Volker Braun

Looks good to me. Are you making a full lcalc spkg at one point?

comment:2 Changed 11 years ago by fbissey

  • Cc fbissey added

comment:3 in reply to: ↑ 1 Changed 11 years ago by jdemeyer

Replying to vbraun:

Looks good to me. Are you making a full lcalc spkg at one point?

Yes, when I have time.

comment:4 Changed 11 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 11 years ago by jdemeyer

  • Dependencies set to #11130

comment:6 Changed 11 years ago by jdemeyer

  • Report Upstream changed from N/A to Reported upstream. Little or no feedback.

comment:7 Changed 11 years ago by jdemeyer

  • Status changed from new to needs_review

New spkg ready for review, needs to be built with the new PARI spkg.

comment:8 Changed 11 years ago by jdemeyer

  • Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.

On 2011-05-12 18:56, Michael Rubinstein wrote:

Thanks for the patch.

I should point out that I now have a google code page for lcalc:

http://code.google.com/p/l-calc/

(lcalc was taken, so I went with l-calc).

The page has my beta newer version of lcalc, L-1.3 However, Rishikesh's cython wrapper is still being updated for that, so it won't work within sage with the current wrapper.

I should point out that Lcommandline_elliptic.cc has become part of the underlying class library and moved to Lelliptic.cc in version L-1.3

Mike

comment:9 Changed 11 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:10 Changed 11 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:11 Changed 11 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 11 years ago by leif

  • Description modified (diff)

The patch to the Makefile still uses -lmpir instead of -lgmp. (We always build MPIR with GMP compatibility, as all other packages refer to GMP rather than MPIR.) And the receipt for all is still funny, but that's upstream...

I'd quote $patch to be on the safe side. Also, the patches are / have to be applied with -p1 (from lcalc-*/src/) rather than -p0, which I don't mind, but Jeroen wanted to unify this across spkgs IIRC.

comment:13 Changed 11 years ago by leif

  • Reviewers changed from Volker Braun to Volker Braun, Leif Leonhardy

spkg-install:

  • The comment and a message regarding SAGE_DEBUG is not consistent. (SAGE_DEBUG has to be set to yes to enable debugging support, while anything else disables it, though not deleting any user-specified flags of course.) I'd always add -g anyway, since this doesn't have any measureable performance impact, only the object files will grow slightly.
  • $CC must not be quoted in the compiler test (if we support flags within CC rather than compiler paths containing spaces). Perhaps a matter of taste, but this should be consistent in Sage, and e.g. distutils uses the former variant, not to mention the way we use $MAKE. Same applies to $CXX and in principle $SAGE_FORTRAN, while the Fortran compiler test is useless there anyway.
  • Funny that Dave uses test -e for the Fortran library...
  • I wonder if error messages etc. should be redirected to stderr as we started to do in a couple of scripts. The disadvantage of doing so is that both streams occasionally get out of sync due to buffering, leading to potentially confusing output.
  • Copy sage specific modifications should now of course read Apply Sage ....
  • $UNAME should also be quoted (twice); case ... in ... would anyway be better.
  • I'm not sure if Cygwin needs both a .dll and a .so file (cf. #11547). Currently the .dll is copied to an .so in $SAGE_LOCAL/lib/. Unless it is statically linked, the stand-alone lcalc might also require the shared library in $SAGE_LOCAL/bin/.

SPKG.txt:

  • The Upstream contact section should be moved up.
  • Special Build Instructions should read ... Update/Build? ....
  • The Changelog heading is missing.
  • The Dependencies section is completely missing.

Some files from src/include/ can also be deleted (and I doubt we have to copy all of them to $SAGE_LOCAL/include/lcalc/; L*.h and mpfr_mul_d.h -- included by Lgmpfrxx.h -- should suffice):

~/Sage/spkgs/lcalc-1.23.p7$ ls src/include/
cmdline.h                      Lfind_zeros.h
getopt.h                       Lgamma.h
Lcommandline_elliptic.h        Lglobals.h
Lcommandline_globals.h         Lgmpfrxx.h
Lcommandline.h                 Lgram.h
Lcommandline_misc.h            L.h
Lcommandline_numbertheory.h    Lint_complex.h
Lcommandline_twist.h           Lmisc.h
Lcommandline_values_zeros.h    Lnumberzeros.h
Lcommon.h                      Lnumeric.h
Lcommon_ld.h                   Lprint.h
Lcomplex.h                     Lriemannsiegel_blfi.h
Ldirichlet_series.h            Lriemannsiegel.h
Ldokchitser.h                  Lvalue.h
Lexplicit_formula.h            Lvalue.h.bak
Lexplicit_formula.h.swap.crap  mpfr_mul_d.h

I wonder if it's worth to run Lcalc's tests from an spkg-check file.


I'd really appreciate having the changes to spkgs to be "finally" reviewed committed, especially if files get deleted, as is the case here. Also, IMHO commit messages are subject to review as well, no matter if some merge script would add a generic one in case it is missing.

comment:14 Changed 11 years ago by jdemeyer

  • Description modified (diff)

New spkg, same location. This cases care of most of leif's comments. I believe that the remainder of leif's comments should not prevent a positive review for this ticket.

About committing the changes: I find it very annoying to have to commit the changes everytime I make a change. If you are happy with the spkg except for the committing of the changes, I can still commit the changes at that point.

Note: See TracTickets for help on using tickets.