Re: [AMBER-Developers] Proposal for a new git branch

From: Ross Walker <ross.rosswalker.co.uk>
Date: Sat, 12 Nov 2011 13:51:00 -0800

Hi All,

> No, that was not my intention. Testing of the master branch would
> continue as at present. But breaking something would not be viewed as
> such a terrible thing as it is now by some people. I did write that
> updating master would imply a commitment to follow the cruise-control
> tests,
> and to promptly fix any problems that it reveals.

I should probably caveat what I wanted to get across here. Firstly the point
of the cruise control server is that it does the compilation testing for
you. Hence you don't have to test on all these different compilers etc
before committing. It will also, eventually, run all the test cases for you
AND keep track of benchmarks. In this way we will all be able to see when
the build was broken, how it was broken, what tests failed, when they failed
and also if performance has been degraded over time. I think this is a good
thing and I would encourage everyone to make use of it. What I want is for
people to check in their work more knowing that this is there and that it
will test things for them and then they can come back in a few hours and see
if they broke it and hopefully fix it.

What I want to put a stop to is where people break the tree, or the test
cases and just don't care. This is what we need to avoid at all costs. In
particular the test cases. We can complain all we want about coding
practices etc but I think the real issue that we should be discussing is
testing of scientific software which is in general appalling these days. I
think the pressure to publish these days results in people really not caring
about whether their code (and thus their research) is actually correct or
not. Now I am not accusing the AMBER community here, in fact I think we as
general do an excellent job of testing and validating our code BUT I think
if we don't keep some discipline in the testing we could run the risk of
heading down a dark path that it will be hard to come back from.

Take the Leap modifications for example. We ALL rely on Leap to generate us
prmtops that are correct for the force field. Thus this is a vital piece of
code that we have to get right. So when people add to this code why are they
not checking by hand that it is producing the correct results and then
creating test cases to provide regression testing of their changes? - Saying
that there are no existing test cases is not an excuse - these should have
been added (using the released version of AMBER 11) at the time the new
functionality was added to make sure nothing was broken.

So I think that is the key. It is not that people might break the build or
the test cases. It is that people are made aware that they broke things and
thus can take a responsibility to fix them.

I see no reason to split things off into what is effectively a 'dirty
hackathon' branch and a 'clean' branch. Instead we should be encouraging
people to check things into the tree so problems can be caught early. BUT
the people doing this have to be prepared to fix things if they break stuff
rather than just dumping in code and disappearing off into the night.

Look at the ConstantPH tests for example. These are broken now and we have
no idea when they started breaking and why. Instead we just get them
disabled... Fail!

Same goes for the MMPBSA test results in AMBER 11 + AmberTools 1.5. Why do
the test cases fail? - Is this rationalized? - simply a different random
number stream? Or did a bunch of bugs get fixed in the new MMPBSA? As an end
user this is VERY alarming (forget the message at the end of AT15_Amber.py -
nobody reads that and it doesn't matter if you have it in day glo green neon
it still won't get read). Instead the test case results should have been
carefully checked and then updated as part of the release. Then we wouldn't
get users posting to the list all concerned because they see a bunch of
failures.

So, let's all get checking our stuff into the tree WITH 'VALIDATED' TES
CASES. Then we can start seeing what has been broken etc.

> For example, cruise control tests 5 variants of Amber and 4 of
> AmberTools.
> My fear is that if we (implicitly) say "you have to run all these tests
> before
> committing to master" we'll miss a lot of good commits, and people will
> retreat to their private branches and not share code.

No the complete opposite as Andreas realized the other day. You can test
your code at least works on your system (I encourage people not to get
totally lazy) and then check it in and within an hour you will have it
tested on various different variants. Then you can go and fix anything you
inadvertently broke.

My point is we should let people know compilation is broken and test cases
fail early so we can track down the problem quickly rather than finding out
6 months down the line just before release.

> Of course, when we get a flurry of last-minute commits, as is typical
> late
> in the development cycle, we may need to ask people to be more careful,
> or use
> some other model.

We can avoid this if people check in regularly, find out what they broke and
then fix it.
 
> [Aside: cruise control has real limits if something fails. I tried to
> see
> why the parallel make was failing, but the "build log" on cruise
> control is
> not human-readable. So, when a red light appears on the dashboard, it
> may be
> best just to try to reproduce the problem on your local machine.]

I am working on this. I am going to try to split it into standard out and
standard error and try to remove unnecessary info. For the moment though the
Build Log is not what you want to look at. You should look at the "Errors
and Warnings Section" which essentially contains the output from Make.

http://git.ambermd.org:8080/dashboard/tab/build/detail/Amber_serial_gnu-4.4.
6-j4

(This will get a LOT cleaner if we start working to clean the code so we
don't get compiler warnings - such as removing the use of GOTO - which is
now a Deleted feature and not just a deprecated one).

But for now the error is:

...
ar: creating sys.a
ar: creating libsqm.a
ar: creating blas.a
ar: fftwf77.o: No such file or directory
make[4]: *** [libdfftw.la] Error 1
make[3]: *** [install-recursive] Error 1
make[2]: *** [-ldrfftw] Error 2
make[2]: *** Waiting for unfinished jobs....
mv: cannot stat `.deps/frc_1.Tpo': No such file or directory
make[3]: *** [frc_1.lo] Error 1
make[2]: *** [install-recursive] Error 1
make[1]: *** [-ldrfftw] Error 2
make[1]: *** Waiting for unfinished jobs....
mv: cannot move `.deps/frc_7.Tpo' to `.deps/frc_7.Plo': No such file or
directory
make[4]: *** [frc_7.lo] Error 1
make[3]: *** [install-recursive] Error 1
make[2]: ***
[-L/home/cc/cruisecontrol-bin-2.8.4/projects/Amber_serial_gnu-4.4.6-j4/lib]
Error 2
make[1]: *** [librism] Error 2
ar: creating lapack.a
dseupd.f:620.72:
...

So it is something in the FFTW build that is stopping make -j4 from working.
Make -j4 inside FFTW manually works so I suspect it is in the building of
Sander.RISM where make sends off one thread to do FFTW while continuing to
compile within src/sander and this leads to the link occurring before the
FFTW compile has completed.

All the best
Ross


/\
\/
|\oss Walker

---------------------------------------------------------
| Assistant Research Professor |
| San Diego Supercomputer Center |
| Adjunct Assistant Professor |
| Dept. of Chemistry and Biochemistry |
| University of California San Diego |
| NVIDIA Fellow |
| http://www.rosswalker.co.uk | http://www.wmd-lab.org/ |
| Tel: +1 858 822 0854 | EMail:- ross.rosswalker.co.uk |
---------------------------------------------------------

Note: Electronic Mail is not secure, has no guarantee of delivery, may not
be read every day, and should not be used for urgent or sensitive issues.





_______________________________________________
AMBER-Developers mailing list
AMBER-Developers.ambermd.org
http://lists.ambermd.org/mailman/listinfo/amber-developers
Received on Sat Nov 12 2011 - 14:00:03 PST
Custom Search