spkg-configure.m4 for palp

[dimpase]

it should be easy - it's a stable package, with just binary executables and data.

In debian it is called palp

CC: @orlitzky @mkoeppe @enriqueartal @isuruf @vbraun

Component: build: configure

Author: Dima Pasechnik

Branch/Commit: 926c9f3

Reviewer: Michael Orlitzky

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

[dimpase]

Commit: 882b9b8

[dimpase]

Author: Dima Pasechnik

[dimpase]

New commits:

882b9b8

spkg-configure for palp

[dimpase]

Branch: u/dimpase/packages/palpconfig

[orlitzky]

comment:2

The reason I haven't included this in Gentoo yet is because sage builds optimized versions of the class.x, cws.x, nef.x, and poly.x executables -- and then installs them with nonstandard names.

Editing Global.h to define POLY_Dmax is what upstream recommends, but the names of the resulting binaries are always e.g. class.x when you do that. Sage then renames them too e.g. class-4d.x. I don't want to include the nonstandard executables in a distro package just to make sage (which isn't even in the tree yet) happy.

Does anyone know if upstream is still interested in making source improvements? It might be easy to consolidate these into one executable by having each foo.x dispatch to one of several optimized functions rather than having the user (i.e. sage) dispatch to one of several different optimized executables. That depends it being easy to infer POLY_Dmax from the input, of course. Otherwise, we could at least ask that the build system standardize on these names and actually build the optimized executables.

[orlitzky]

comment:3

I think I forgot to write this: in sage/geometry/polyhedron/palp_database.py, we have

        return Popen(['class-4d.x', '-He',

so Sage does use at least use that nonstandard name, and this script needs to check for it. I'm not sure if that will affect any of the other distros.

[dimpase]

comment:4

Debian has essentially the same naming scheme for palp's programs in its package.

Fedora as well.

[orlitzky]

comment:5

Replying to @dimpase:

Debian has essentially the same naming scheme for palp's programs in its package.

Fedora as well.

If all of the major distros are using the same convoluted build process to achieve a consistent naming scheme, then to me that still suggests that we should upstream that build process. It's hard for me to justify building the same package four times and installing a bunch of executables with non-standard names just to make the test suite of another package that's not even in the tree pass.

In lieu of some agreement, I'll probably add a fallback to sagelib so that it uses class.x instead of class-4d.x when the latter isn't present (like we did with nauty). That will allow sage to keep working with the "out of the box" version of palp.

[dimpase]

comment:6

yes, assuming upstream is sensible. I spent a lot of time convincing upstreams to accept patches, it is often frustrating.

[orlitzky]

comment:7

I sent the maintainer a cleanup patch for the makefile, so I'll get some sort of impression soon enough.

[orlitzky]

comment:8

It turns out that the only place class-4d.x is even used is in an optional test for a huge, old-style SPKG that hasn't been ported yet (#26029). The nonstandard names are thus quite likely to be completely unused.

[orlitzky]

comment:9

Just kidding, the names of the executables are mangled on the fly. In geometry/lattice_polytope.py,

def set_palp_dimension(d):
    ...
    global _palp_dimension
    _palp_dimension = d

def _palp(command, polytopes, reduce_dimension=False):
    ...
    if _palp_dimension is not None:
        dot = command.find(".")
        command = command[:dot] + "-%dd" % _palp_dimension + command[dot:]

I give up.

[orlitzky]

comment:10

In light of that, you should check all 16 combinations of executable names (poly, class, nef cws) x (4,5,6 11) in the spkg-configure.m4. I started a branch at u/mjo/ticket/29672 that checks for class-4d.x and substitutes class.x in its place, but I'm not going to spend three days doing it for the other 15 names.

[dimpase]

comment:11

Volker, you're a palp user - could you comment on the naming schemes for palp executables?

[dimpase]

comment:13

Replying to @orlitzky:

In light of that, you should check all 16 combinations of executable names (poly, class, nef cws) x (4,5,6 11) in the spkg-configure.m4. I started a branch at u/mjo/ticket/29672 that checks for class-4d.x and substitutes class.x in its place, but I'm not going to spend three days doing it for the other 15 names.

OK, I'll try my hand in nested m4 loops :-)

[orlitzky]

comment:14

Replying to @orlitzky:

I sent the maintainer a cleanup patch for the makefile, so I'll get some sort of impression soon enough.

Harald Skarke just merged these patches. It probably wouldn't be hard to fix the naming issue if everyone can agree on a solution.

I think having one executable (e.g. class.x) dispatch to the proper routine is clearly preferable, if the dimension can be determined quickly enough to do so. This avoids invalidating the examples in the paper (https://arxiv.org/abs/math/0204356) unnecessarily, and doesn't pollute /usr/bin with four copies of the same programs.

[dimpase]

comment:15

So, what's going to change once these merged upstream patches are in distros?

[orlitzky]

comment:16

Replying to @dimpase:

So, what's going to change once these merged upstream patches are in distros?

Probably nothing, the series I sent only ensures that CC, LDFLAGS, CFLAGS, CPPFLAGS are supported out of the box.

[thierry-FreeBSD]

comment:17

I just made a port of palm for FreeBSD, based upon build/pkgs/palp/spkg-install.in, but I do not see any data in it. The resulting MANIFEST (relative to $PREFIX) is:

bin/class-11d.x
bin/class-4d.x
bin/class-5d.x
bin/class-6d.x
bin/class.x
bin/cws-11d.x
bin/cws-4d.x
bin/cws-5d.x
bin/cws-6d.x
bin/cws.x
bin/mori-11d.x
bin/mori-4d.x
bin/mori-5d.x
bin/mori-6d.x
bin/mori.x
bin/nef-11d.x
bin/nef-4d.x
bin/nef-5d.x
bin/nef-6d.x
bin/nef.x
bin/poly-11d.x
bin/poly-4d.x
bin/poly-5d.x
bin/poly-6d.x
bin/poly.x

[dimpase]

comment:18

one way to test palp executables is to look up examples in
https://arxiv.org/abs/math/0204356

[thierry-FreeBSD]

comment:19

Yes, it works!

But my question was about the description of this ticket "it's a stable package, with just binary executables and data", since I do not see any datafiles.

[sagetrac-git]

Changed commit from 882b9b8 to c79fc1b

[sagetrac-git]

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

700ce3b	spkg-configure for palp
c79fc1b	inner loop macro

[sagetrac-git]

Changed commit from c79fc1b to 4a6ec70

[sagetrac-git]

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

4a6ec70

getting quoting right everywhere

[dimpase]

comment:22

OK, so as requested, checking that every executable is there. Needs review (you can just look in the config.log to see it does the right thing)

[orlitzky]

comment:23

The logic is OK but the variable tests need quotes otherwise they crash if I do something stupid like put the executables in /home/mjo/My Programs.

[orlitzky]

Reviewer: Michael Orlitzky

[sagetrac-git]

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

926c9f3

added quotes

[sagetrac-git]

Changed commit from 4a6ec70 to 926c9f3

[dimpase]

comment:25

Replying to @orlitzky:

The logic is OK but the variable tests need quotes otherwise they crash if I do something stupid like put the executables in /home/mjo/My Programs.

OK, sure, done.

[orlitzky]

comment:26

Ok, works now.

[vbraun]

Changed branch from u/dimpase/packages/palpconfig to 926c9f3