#853: Add a pslq implementation to Sage
--------------------------------------------+-------------------------------
Reporter: was | Owner: was
Type: enhancement | Status: needs_info
Priority: major | Milestone: sage-wishlist
Component: number theory | Keywords:
Author: Paul Zimmermann, Alex Ghitza | Upstream: N/A
Reviewer: David Kirkby | Merged:
Work_issues: need advice on interface |
--------------------------------------------+-------------------------------
Changes (by drkirkby):
* reviewer: => David Kirkby
Comment:
It's good to see you have tested this on Solaris, though there is a
potential Solaris issue:
{{{
if [ `uname` = "Darwin" -a "$SAGE64" = "yes" ]; then
echo "64 bit MacIntel"
CFLAGS="$CFLAGS -m64 "; export CFLAGS
fi
}}}
should, at the very least, be replaced by
{{{
if [ "x`uname`" = xyes ]; then
echo "Building a 64-bit version of pslq"
CFLAGS="$CFLAGS -m64 "; export CFLAGS
LDFLAGS="$LDFLAGS -m64 "; export LDFLAGS
fi
}}}
It is in general safer to put an x in front of the `uname` and then test
for whatever we want, with an x in front of that. (This is for maximum
portability, using any shell, and is not essential, but I think a good
habit to get into). There is no need to quote "xyes" as we know it has no
spaces in it.
Without removing the Darwin restriction, it would be impossible to build a
64-bit version on Solaris, !OpenSolaris or other system such as HP-UX
where both 32-bit and 64-bit executables are supported.
Whether LDFLAGS is necessary or not depends on the package. I've not tried
building this 64-bit Solaris. I'll have a look at that later, but I do not
think setting LDFLAGS ever appears to do any harm, and is sometimes
essential.
Actually, better still would be
{{{
if [ -z "$CFLAG64" ] ; then
CFLAG64=-m64
fi
if [ "x`uname`" = xyes ]; then
echo "Building a 64-bit of pslq"
CFLAGS="$CFLAGS $CFLAG64"
LDFLAGS="$LDFLAGS $CFLAG64"
fi
if [ "x`$SAGE_LOCAL/bin/testcc.sh`" = xGNU ] ; then
CFLAGS="$CFLAGS -Wall -pedantic"
fi
export CFLAGS
export LDFLAGS
}}}
as that would
* Allow the variable CFLAG64 to be set to whatever compiler flag is
necessary to build 64-bit code, which is not -m64 for all compilers.
(CFLAG64 has been used in other packages for this purpose, to increase
portability).
* Add the compiler options -Wall and -pedantic if using gcc.
Compiling with the -Wall -pedantic options I get:
{{{
drkir...@hawk:/tmp/pslq-1.0/src$ gcc -Wall -pedantic -c pslq-1.0.c
pslq-1.0.c: In function ‘print_column’:
pslq-1.0.c:175: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long unsigned int’
pslq-1.0.c: In function ‘print_relation’:
pslq-1.0.c:224: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 3 has type ‘long unsigned int’
pslq-1.0.c: In function ‘print_matrix’:
pslq-1.0.c:240: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long unsigned int’
pslq-1.0.c: In function ‘pslq’:
pslq-1.0.c:855: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long int’
pslq-1.0.c:858: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long int’
pslq-1.0.c:860: warning: format ‘%d’ expects type ‘int’, but argument 2
has type ‘long int’
pslq-1.0.c:870: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long int’
pslq-1.0.c:892: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long int’
pslq-1.0.c: In function ‘main’:
pslq-1.0.c:972: warning: implicit declaration of function ‘strcmp’
pslq-1.0.c:1040: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long unsigned int’
pslq-1.0.c:1044: warning: format ‘%u’ expects type ‘unsigned int *’, but
argument 2 has type ‘long unsigned int *’
pslq-1.0.c:1055: warning: format ‘%u’ expects type ‘unsigned int’, but
argument 2 has type ‘long unsigned int’
}}}
Some of those warnings would lead me to believe a 64-bit build of this
would not work as expected. In that case, 'unsigned int' would be 4 bytes,
but 'long unsigned int' would be 8 bytes. That could go very pear shaped.
The function strcmp() is defined in strings.h on Solaris, so I would
suggest adding
{{{
#include <strings.h>
}}}
I can't comment on the maths aspect of it - I'm not a mathematician.
Some of these issues need reporting upstream, some are problems with spkg-
install.
'''I would note that all of the above code snippets I wrote were untested,
so would need testing'''
Dave
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/853#comment:11>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
