lcalc code quality
Volker Braun
Jeroen Demeyer
> Is anybody working on either removing lcalc or getting its code up to
> reasonable quality? Does anybody know the upstream author? The lcalc
> spkg recently broke on gcc-4.6 essentially due to this:
>
> #define double(x) (double)(lcalc_to_double(x))
>
> I mean, WTF?
I agree with the WTF...
Robert Bradshaw
> Is anybody working on either removing lcalc or getting its code up to
> reasonable quality?
Is it not giving you the right answers anymore?
> Does anybody know the upstream author?
Yes, several of us.
> The lcalc spkg
> recently broke on gcc-4.6 essentially due to this:
> #define double(x) (double)(lcalc_to_double(x))
>
> I mean, WTF? Does anybody believe that you can safely include system headers
> after redefining cast-to-double by a preprocessor macro? The lcalc code must
> be refactored to not rely on this construction. As Dave Kirby remarked,
> right now it compiles on a couple of gcc releases and essentially no other
> compiler. And thats hardly a surprise if you look at the code. At the very
> least the endless compiler warnings need to be looked at / fixed.
The state of this spkg has been discussed, e.g.:
http://groups.google.com/group/sage-devel/browse_thread/thread/b674cf0bc8e1eb70/00042695e9daf61a#00042695e9daf61a
Yes, there's room for improvement, and Mike Rubinstein *has* spent
time cleaning up his code. Lets fix it rather than hashing over this
yet again.
- Robert
John Cremona
me right now at MSRI. But I would not like to forward any emails to
him which were not a lot more polite.
John
> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org
>
Volker Braun
On Thu, Mar 31, 2011 at 9:22 AM, Volker Braun <vbraun.name@gmail.com> wrote:
> Is anybody working on either removing lcalc or getting its code up to
> reasonable quality?Is it not giving you the right answers anymore?
kcrisman
> And I agree that we should communicate with the author politely. I was
> addressing the Sage developers that use lcalc, and I think that its
> allowable to use a more colloquial tone in that case.
discussing component pieces of Sage. We depend on them to make it
work great. This is especially true when discussing something fairly
technical to a large proportion of Sage developers, I would guess, and
certainly to the many users who subscribe or visit this on the web in
the hopes of learning something. This is a public forum, even if it
often doesn't seem like it because we get to know one another to some
extent at Sage Days and on the list.
As a separate issue, I realize that not everyone will agree on what
acronyms/cuss words are appropriate for this forum, so I won't push
this too strongly. But my guess is that the public Mma lists, or
Mapleprimes, wouldn't be too hot on this as a customer relations
strategy.
(I also realize that now someone will prove me wrong by finding a much
worse example on one of those sites!)
Colloquially yours,
- kcrisman
Jason Grout
>
>> And I agree that we should communicate with the author politely. I was
>> addressing the Sage developers that use lcalc, and I think that its
>> allowable to use a more colloquial tone in that case.
>
> I would submit that we should be as polite as possible whenever
> discussing component pieces of Sage. We depend on them to make it
> work great. This is especially true when discussing something fairly
> technical to a large proportion of Sage developers, I would guess, and
> certainly to the many users who subscribe or visit this on the web in
> the hopes of learning something. This is a public forum, even if it
> often doesn't seem like it because we get to know one another to some
> extent at Sage Days and on the list.
>
> As a separate issue, I realize that not everyone will agree on what
> acronyms/cuss words are appropriate for this forum, so I won't push
> this too strongly. But my guess is that the public Mma lists, or
> Mapleprimes, wouldn't be too hot on this as a customer relations
> strategy.
+1 to everything you said. Also, I'd like to point out that since many
upstream developers lurk on this and other Sage lists, our "colloquial"
conversations actually are heard by many upstream developers. Even if a
particular upstream developer is not subscribed, many other upstream
developers see how we behave as a group and that will probably influence
their opinion of Sage.
Thanks,
Jason
Dr. David Kirkby
>As Dave Kirby remarked,
> right now it compiles on a couple of gcc releases and essentially no other
> compiler. And thats hardly a surprise if you look at the code. At the very
> least the endless compiler warnings need to be looked at / fixed.
Micheal did a while back say:
"The antiquated header files and unused variables have been removed in L-1.3,
which has not yet been released to the public. It should be released in the
next couple of weeks"
I don't know if he has had time to address that. Are we using an old version?
Perhaps he has released 1.3. I can't find it I must admit, but nearly all lcalc
references on Google seem to be related to Sage.
One thing that might help him improve the code is to run it though the Sun
Studio compiler, which can be downloaded and installed on Linux, either as an
rpm or a tar file which does not need root access to install.
http://www.oracle.com/technetwork/server-storage/solarisstudio/downloads/index.html
That is more strict that gcc, and would pick up some of these problems. Perhaps
he is not aware of that.
Could someone install that on sage.math? If code will compile that Sun Studio,
and compile with gcc version n, we can be pretty sure it will compile with gcc
n+1. Clearly we are finding code which is breaking as gcc gets stricter, whereas
the Sun compiler would never have accepted it in the first place.
Singular, PolyBoRi and lcalc are three packages which have become broken when
updating from gcc 4.5.x to 4.6.0
I recently played with a static analysis tool, cppcheck:
http://sourceforge.net/apps/mediawiki/cppcheck/index.php?title=Main_Page
which is might be of use too, though I was not over-impressed. I tried it on
some of my own code, but whilst it found some issues, all were non-issues. It
did not find any problems at all with lcalc. It also generates a seg fault after
it has finished running
Checking ./Lglobals.cc: USE_MPFR...
10/15 files checked 66% done
Checking ./Lmisc.cc...
11/15 files checked 73% done
Checking ./Lriemannsiegel.cc...
12/15 files checked 80% done
Checking ./Lriemannsiegel_blfi.cc...
13/15 files checked 86% done
Checking ./cmdline.c...
Checking ./cmdline.c: HAVE_CONFIG_H...
14/15 files checked 93% done
Checking ./example_programs/example.cc...
15/15 files checked 100% done
Segmentation Fault (core dumped)
which is not a good advert for a piece of code designed to check code!!
There are some similar commercial tools.
But really cleaning up the compiler warnings and getting standard C or C++ would
be the first step. Static code analysis tools could come later, when the code is
cleaner.
--
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?
Dave
John Cremona
Mike is certainly very busy now. (In the last half hour I have been
receiving alternate emails from him, and from sage-devel about
lcalc!). He is (co-)running the current semester at MSRI on
Arithmetic Statistics (other organisers including me, and William).
He has a newborn baby keeping him awake at night. And he is working
hard at getting the large (and well-funded) L-functions database
project ready for release later this year, one of the important
activities currently happening at MSRI.
A lot of work on Sage and attendance at Sage Days has been funded by
Mike's FRG L-functions grant (for which William is a
co-investigator).
Just this week Rishi was demonstrating (at MSRI) his recent work on
improving the Sage interface to both lcalc and Dokchitser's L-function
code. These issues about precision came up then.
I don't think the expression "upstream author" quite captures the
relationship here. And I agree with everything Karl-Dieter said too.
John
Francois Bissey
> On 03/31/11 05:22 PM, Volker Braun wrote:
> >As Dave Kirby remarked,
> >
> > right now it compiles on a couple of gcc releases and essentially no
> > other compiler. And thats hardly a surprise if you look at the code. At
> > the very least the endless compiler warnings need to be looked at /
> > fixed.
>
> Micheal did a while back say:
>
> "The antiquated header files and unused variables have been removed in
> L-1.3, which has not yet been released to the public. It should be
> released in the next couple of weeks"
>
Yes I had that same reply to a private message I sent him some time ago.
I had tried to fix the deprecated header warning but it broke sage's tests
in turn. They broke because with my fix lcalc couldn't take input that were
as large as before...
I'll certainly commiserate with him being busy with his new baby (got a new
one of my own on Monday, they are demanding little thing, let's not mention
the students...)
Francois
This email may be confidential and subject to legal privilege, it may not reflect the views of the University of Canterbury, and it is not guaranteed to be virus free. If you are not an intended recipient, please notify the sender immediately and erase all copies of the message and any attachments. Please refer to http://www.canterbury.ac.nz/emaildisclaimer for more information.
John Cremona
<francoi...@canterbury.ac.nz> wrote:
> I'll certainly commiserate with him being busy with his new baby (got a new
> one of my own on Monday, they are demanding little thing, let's not mention
> the students...)
Congratulations!
John
Volker Braun
I realize that not everyone will agree on what
acronyms/cuss words are appropriate for this forum
Francois Bissey
An idea that may be on the constructive side.
John could you convince/help Micheal to upload his current code
in a hg repo on google code like you did with eclib?
Rather than whining we could contribute and take a snapshot
when it is satisfactory.
Francois
Robert Bradshaw
+1, to all of the above.
What I was trying to say is that we should focus on getting the (very
sophisticated and useful) code to compile and work again, rather than
complain about the number of compiler warnings it gives. Getting it
into a public repository would be a great step forward for those who
care about cross-platform compatibility and compiler workings to
contribute.
- Robert
Dr. David Kirkby
> +1, to all of the above.
>
> What I was trying to say is that we should focus on getting the (very
> sophisticated and useful) code to compile and work again, rather than
> complain about the number of compiler warnings it gives. Getting it
> into a public repository would be a great step forward for those who
> care about cross-platform compatibility and compiler workings to
> contribute.
>
> - Robert
A possible (probable?) problem will be that it will be very difficult for
someone to know what changes to make. The current lcalc code in Sage is not
valid C++, so trying to know what the author intended would be difficult and I
feel attempting to clean it up would be more likely to break the code than
improve it.
BTW, what is the full name of the author? SPKG.txt says "Rishikesh", yet people
here talk of "Risi" and "Mike".
I've fixed many portability issues in the past, when it was clear what the
author intended and one can see his mistakes. This is probably not so with
'lcalc' - at least based on what I can see just now.
Obviously if revised code is much better then it may be possible for someone to
clean it up further.
Sympow was another case similar. I don't think anyone could clean up Sympow, as
it's not valid C, so hard to know what the author was trying to do. I only
managed to fix a bug in that as a result of finding a reference to a paper,
reading the paper, then I was able to work out what the author intended. But I
believe cleaning up Sympow would be dangerous rather than constructive.
The big advantage here with lcalc is the author is still available, although
busy just now. It might be safer to leave him to do it. If the Sun compiler was
installed, this would give him the chance to turn it into valid C++. Then it
would be much more maintainable.
Dave
John Cremona
> BTW, what is the full name of the author? SPKG.txt says "Rishikesh", yet
> people here talk of "Risi" and "Mike".
>
Rishikesh is Mike Rubenstein's graduate student. That is his entire
name; Rishi is an abbreviation. Mike is an abbreviation of Michael.
I will suggest to Mike within the next hour about putting lcalc onto
googlecode, just as I did with eclib.
In fact, what made me put eclib onto google code was following Mike's
example http://code.google.com/p/lmfdb/
John
Bill Hart
Please also bear in mind that many "upstream" developers have put
years of their life into research, development of algorithms and
coding. Many of them are professional mathematicians, not computer
scientists or professional programmers. They live and die by theorems,
grants, teaching, publications, etc., not language standards and may
only care if their code works for them and their immediate friends!
They very often do not have time to maintain it to the high standard
they might prefer, and their donation of their code is made on that
understanding.
There's a fine line between critiquing/refereeing/improving code and
ridiculing it.
If you want to ridicule code which does not conform to the standard,
take your best shot:
http://stackoverflow.com/questions/4568645/printing-1-to-1000-without-loop-or-conditionals/4583502#4583502
David Kirkby
> Please also bear in mind that many "upstream" developers have put
> years of their life into research, development of algorithms and
> coding. Many of them are professional mathematicians, not computer
> scientists or professional programmers. They live and die by theorems,
> grants, teaching, publications, etc., not language standards and may
> only care if their code works for them and their immediate friends!
> They very often do not have time to maintain it to the high standard
> they might prefer, and their donation of their code is made on that
> understanding.
Fair enough, but I would hope the quality control in Sage would
prevent poor code being merged in the first place. That does not seem
to have been so, though I think the situation is improving somewhat.
I would not expect to see code written by pilots controlling aircraft
or code written by medical staff controlling medical equipment.
OK, these are extreme cases, but happen to be two industries I have
worked in (aeronautical and medical), though I have never had to write
safety-critical software for either industry.
> There's a fine line between critiquing/refereeing/improving code and
> ridiculing it.
Very true, but the fact it is in Sage, does cause frustration. I
suspect it was frustration which was the provocation for the initial
rather harsh comments on this thread.
There are bits of code in Sage, which even a cursory glance at the
source code, or a look at the compiler warnings, should have in my
opinion stopped it being in Sage before it was cleaned up.
If the code was thought to be very useful (like Sympow), but of
dubious quality (you yourself once wrote Sympow was "virtually
obfuscated")
https://groups.google.com/group/sage-devel/msg/d7aef307c15de2e6?hl=en
then perhaps it should have remained experimental. Or perhaps it
should have never been put in Sage, but people used it external to
Sage.
Dave
Robert Bradshaw
> On 2 April 2011 14:20, Bill Hart <goodwi...@googlemail.com> wrote:
>
>> Please also bear in mind that many "upstream" developers have put
>> years of their life into research, development of algorithms and
>> coding. Many of them are professional mathematicians, not computer
>> scientists or professional programmers. They live and die by theorems,
>> grants, teaching, publications, etc., not language standards and may
>> only care if their code works for them and their immediate friends!
>> They very often do not have time to maintain it to the high standard
>> they might prefer, and their donation of their code is made on that
>> understanding.
>
>
> Fair enough, but I would hope the quality control in Sage would
> prevent poor code being merged in the first place. That does not seem
> to have been so, though I think the situation is improving somewhat.
I think the key point is that there are several metrics for judging
code. Some judge code by how many compiler warnings it produces, or
how standard-abiding it is. Others judge code by clarity of the
expressed algorithm(s). Yet others judge code by whether it works
correctly for their domain, or the sophistication of the algorithm, or
its speed. All of us weight these factors differently, and I don't
think anyone would consider any of these criteria bad, but we
shouldn't say code is bad because it doesn't satisfy only our choice
of metric.
> I would not expect to see code written by pilots controlling aircraft
> or code written by medical staff controlling medical equipment.
>
> OK, these are extreme cases, but happen to be two industries I have
> worked in (aeronautical and medical), though I have never had to write
> safety-critical software for either industry.
>
>> There's a fine line between critiquing/refereeing/improving code and
>> ridiculing it.
+1
- Robert
Volker Braun
I think the key point is that there are several metrics for judging
code.
Justin C. Walker
On Apr 2, 2011, at 06:20 , Bill Hart wrote:
>
>
> On Mar 31, 8:41 pm, Jason Grout <jason-s...@creativetrax.com> wrote:
>> On 3/31/11 2:09 PM, kcrisman wrote:
>
>> +1 to everything you said. Also, I'd like to point out that since many
>> upstream developers lurk on this and other Sage lists, our "colloquial"
>> conversations actually are heard by many upstream developers. Even if a
>> particular upstream developer is not subscribed, many other upstream
>> developers see how we behave as a group and that will probably influence
>> their opinion of Sage.
> There's a fine line between critiquing/refereeing/improving code and
> ridiculing it.
1++
--
Justin C. Walker, Curmudgeon at Large
Director
Institute for the Enhancement of the Director's Income
-----------
Nobody knows the trouble I've been
-----------
David Kirkby
> I think the key point is that there are several metrics for judging
> code. Some judge code by how many compiler warnings it produces, or
> how standard-abiding it is. Others judge code by clarity of the
> expressed algorithm(s).
It's hard for me to see how code that produces loads of compiler
warnings about unused variables can offer clarity of the algorithm. If
you find a variable called "cutoff" which is not used, one is never
sure if the code for the cutoff was overlooked, or whether it was not
needed. (There was, perhaps still is, a bit of code in Sage which has
an unused variable called "cutoff")
> Yet others judge code by whether it works
> correctly for their domain, or the sophistication of the algorithm, or
> its speed.
I understand all those. I feel if the speed is slow, if there's
nothing better that's open-source, then I don't have a problem with
it. Within reason, functionality that is slow is better than no
functionality.
> All of us weight these factors differently, and I don't
> think anyone would consider any of these criteria bad, but we
> shouldn't say code is bad because it doesn't satisfy only our choice
> of metric.
Agreed.
But I agree with Volker's comments he posted a day or so ago about
code relying on specific header files or compiler features.
Such code may be perfectly acceptable for an individual to use on his
or her computer, but its incorporation into software that designed to
be portable is not a good idea until such time as either an upstream
author or a Sage developer has cleaned it up. The particular software
under discussion has been the source of several issues I'm aware of:
1) Passing of an assembler option to hide warnings caused a problem on Solaris.
http://trac.sagemath.org/sage_trac/ticket/6609
2) This gcc 4.6 issue
http://trac.sagemath.org/sage_trac/ticket/10892
3) A Cygwin issue (still unresolved) giving no results
http://trac.sagemath.org/sage_trac/ticket/9165
4) Fails to build on HP-UX (admittedly not a high priority platform)
http://trac.sagemath.org/sage_trac/ticket/7178
There are a number of tell-tale signs which should have ensured the
code had an overhaul before being merged into software like Sage.
Currently the latest gcc (4.6.0) is a bit more fussy than previous
versions, so 3 pieces of software in Sage have been broken (Singular,
Lcalc and PolyBoRi). Lcalc has a workaround, PolyBoRi has been fixed
properly and to the best of my knowledge, the Singular problem has not
been resolved.
Dave
Bill Hart
are not expert coders to contribute code to Sage.
Perhaps this could be documented somewhere.
On Apr 2, 4:18 pm, David Kirkby <david.kir...@onetel.net> wrote:
kcrisman
algorithms/coding them, and all the hoo-ha you have to do to get a
computer to do that fast in compiled languages, headers and whatnot.
I realize those things are important, but they are almost certainly
the last thing that a classically-trained mathematician is likely to
see.
It could conceivably be worth having a few people who know what they
are talking about with such things (many contributors to this thread)
make some short videos which could be presented at Sage Days or online
that explain some of the issues that have been raised "from the ground
up". Or point to well-documented, step-by-step, tutorials for such
things. Unfortunately, many internet references read like man pages -
designed for people who already know how it works. Anyway, that could
be an easy place to point for future issues of this type.
- kcrisman
Volker Braun
Ah, it seems that you do not actually want expert mathematicians who
are not expert coders to contribute code to Sage.
William Stein
> On Tuesday, April 5, 2011 8:13:17 AM UTC-4, Bill Hart wrote:
>>
>> Ah, it seems that you do not actually want expert mathematicians who
>> are not expert coders to contribute code to Sage.
>
> You are missing the point of this thread; its not at all about Mike R
> tripping over one of the numerous C++ pitfalls. I don't doubt that his
> program is great at what it does, and its awesome that he shares it with
> us.
> The issue is what I see as a failure of the Sage review process where some
> code that doesn't seem to be ready for public consumption was shoehorned
> into a library.
There was no Sage review process when lcalc was added (by me) to Sage.
At the time (2005?) I did not know that Sage would grow beyond just
being a number theory system used mostly by me.
A new substantial upstream library that has tons of issues should
definitely not be added to Sage today, and I don't think any such
thing has happened since 2007 when we enacted a voting/review process
for package inclusion.
> Ideally, one of the Sage developers who works in that field
> would team up with the upstream author and polish lcalc. This would be both
> a service to Sage and the mathematical community.
+1
That would be great, and I'm very much looking forward to it
happening. Mike is normally at Univ of Waterloo, which has an
extremely strong CS program and CS students, so this could perhaps be
a student project sometime in the future. Perhaps this thread will
provide some motivation for such a project.
-- William