On 09/14/10 01:48 PM, Nathann Cohen wrote:
David,
First, I remember having said once on this mailing list -- but perhaps
it was not to you and in this case I do not mind saying it again --
that I mostly disliked to be criticized (for loss of a better word) by
people who hided behind "the developper", "the programmer", or any
name they could find other than mine when they obviously perfectly
know who they are talking about. If they absolutely have to do it,
though, I like to be added as a recipient of the message, and not to
find this out by pure luck on this google group.
I had my suspicions it was you, but I was not sure - I had no intentions of
accusing anyone unless I was sure. I had in fact cc'ed you on the ticket, but
you made no comments whatsoever.
There's good evidence to suggest this code was not entirely yours. There was
code to use scons in the very first commit, but I somewhat doubt you would use
scons. I somewhat doubt you fully understand SCons - you are not alone there!
So I don't know where this code came from.
If by any miracle you still ignored that I wrote this (very poor --
let me add it for you) interface and all the functions of Sage that
use Cliquer, you will find my name under the "SPKG Maintainer" field
of the SPKG.txt file you made me modify not so long ago, which may be
enough to drop me a line when you are asking whether the package
should be removed.
In that case it was you who raised a ticket to make changes - add a URL that was
missing and requested by a Debian developer. I just reviewed it.
So. Now, more practically :
I've looked briefly at what is supposed to be the unmodified Cliquer code, but
there are endless modifications to the upstream source code.
What do you call endless ? 4 functions at the beginning of cl.c, which
all begin by sage_* (and are imported in cliquer.pxd) ?
There are no comments in the source code to say who wrote them.
How do I know what changes (if any) you made to code that does not have "sage"
in the name?
Another change that has been made is to the Makefile to build a shared library.
Again, not commented.
There's nothing in SPKG.txt to indicate you added a shared library.
Do you expect me to be a mind reader?
It has been
one year since I wrote that and I was learning both Cython and how to
contribute to Sage at that time, but I would be glad to answer any
question you could have on this respect.
But NONE of that should be to the upstream source code. How the hell I am
supposed to know what changes you have made?
Sure, some have changes with "sage" in the name, but how does one really know
what else is changed? Am I expected to track down an old version of the code (no
longer on line), and try to work out what changes have been made?
Those modifications look very dubious too. For example, there are endless calls
to malloc, but the programmer has not bothered to check if the calls to malloc
fails or not. Those modifications are in sections specific to Sage, but there
are similar examples in the upstream source code too.
The upstream source code does NOT create a shared library at all, so the *Sage*
developer that has messed around to create a shared library has screwed up.
Perhaps it's just me, but when I see code where people can't be bothered to
check if malloc fails or not, I suspect they have not taken much care elsewhere
too.
I suspect the researcher who first wrote this algorithm spent quite a
lot of time thinking about it (weeks, months ? no idea !), to finally
find an efficient one, and take the time to implement it, and
eventually to use it... In any case I advise you not to judge the work
of a man according to how he deals with his "malloc". Which lead us to
the following question :
Why not? I am perfectly at liberty to judge the code quality based on the lax
use of malloc.
I know you personally have the attitude of "not thinking about how code can
fail, but fixing bugs when reported". But that is *not* the way code should be
written.
If you do not know about how to create a shared library, why do you do so?
As a matter if interest, is the command line tool 'cl' used? If not, a whole
chunk of code can be commented out - code that is possibly the cause of the
library issues.
>> Do we really need Cliquer in Sage?
This is the most difficult part of this email. Peace.
Dear David,
I think we indeed need Cliquer in Sage. It computes maximum cliques,
which is a hard job. It does it well. It is actually used in many
methods already, and I was thinking of letting yet other methods work
on algorithm based on the computation of maximum cliques to make them
even faster. If you have really thought about what you ask, with what
do you intend to replace the features such a removal would create ? Do
you mean that we should just remove all the methods based on them or
multiply their runtime by 100 in the best case ? Surely you are not
just asking to remove a package regardless of what it is used for,
just because you think Sage must be supported on another of your
computers ?
No. I've had issues with lots of packages. But this is clearly one seriously
messed up package.
Please, count my answer as a -1. I think Cliquer is a nice addition in
Sage's Graph Library.
Now David, please consider the amount of time you have spent on this
solaris support. Think of the work you required others to do because
of this ( the "Has this been tested on Solaris" messages you posted on
most TRAC tickets for instance).
William has accepted Solaris support is very important. Someone was actually
paid for several years to do just that.
Just as important is the fact that adding Solaris support tends to find bad code
- like Cliquer, like SYMPOW. (To be fair, Cliquer is not as bad as SYMPOW)
Think about all that had to be
rewritten because of that, think about the time some patches have
required just because of this platform, about -- the very fact that
you are today asking to REMOVE FEATURES from Sage because of Solaris.
Not just because of Solaris
* Because of what I consider to be poor coding practices - allocating memory
and not bothering to check whether that failed or not.
* The upstream code has been altered in a way that's impossible for me to see
what changes have been mode.
Personally, if there was a vote now about including Cliquer, I would be against
it on the ground the code is dubious.
This is mad. I don't even know who is using Solaris. I asked a friend
who answered immediately : OpenSolaris has been dead since Oracle
bought it. What's the point ?
OpenSolaris and Solaris are two very different things. I tend to do most testing
now on OpenSolaris since I have access to much faster hardware. But the issues
are common to Solaris, OpenSolaris, both on SPARC and x86 processors.
This issues is seen on 32-bit Solaris (where it is not fatal) and 64-bit Solaris
(where it is fatal).
IMHO, if you can't be bothered to read the Sage developers guide and read how
.spkg files are supposed to be maintained, then you should not maintain them.
You do a very poor job of doing so, which creates headaches for others.
Nathann
Dave
--
To post to this group, send an email to sage-devel@googlegroups.com
To unsubscribe from this group, send an email to
sage-devel+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
