[gmx-developers] future of tabulated bonded interactions

[Mark Abraham]

Hi,

I agree. My own number of commits has gone down a lot in the last months,
thanks mostly to not having time to write code, either. This is symptomatic
of the same problem - that we are trying to do too many things. It's going
to get worse as more Stockholm people start to address their skills gaps
with C++11. We can either get developer effort from more people (KTH is
likely to hire someone soon, the support from the recent NIH grant will
make several useful contributions that will benefit lots of people), get
non-developer effort from those parts of the community willing to help us
maintain the things that they particularly value, and/or give ourselves a
smaller problem. And I do not think we can cope without doing all three.

Removing features and enabling them later isn't any more work than
maintaining them in a somewhat broken state - the difference is that the
work is being done when someone knows it is useful. I would also much
prefer the reputation of GROMACS to be that it is very good and very
reliable at its core functionality, that it is a good tool upon which to
build, rather than it is a Swiss Army knife, many of whose tools are
somewhat broken in not very predictable way.

On the code review front, I think we need some policy changes there, too.
The care given to correctness, maintainability and performance of core
mdrun functionality isn't warranted for large parts of the rest of the
code. If people do care but can't contribute review or fixing now, they
should be free to say so, and negotiate. But if a change follows a plan
laid out in eg a Redmine, has significant eventual value to end users,
passes Jenkins, follows style, and has test coverage suitable for the
change, it should be able to go through with a single +2. And if nobody
cares in a month even to comment that they care, I think it should be up to
the author to decide it can go in. If/when we see problems, we have version
control and we can revert changes, or adapt them later. We're already
planning to use some aspects of this approach with a post-submit
verification stage in Jenkins.

The key problem with the recent RNG screwup is that there was no high level
test coverage at all, because we regenerated Tue regressiontests values,
without any reviewer suggesting that we first change the code in a way that
didn't break the order of the random number stream. But we'll never have
any high level test coverage with the current rate of review of patches
that add tests.

Mark

On Wed, 1 Jun 2016 19:27 Teemu Murtola <teemu.murtola at gmail.com> wrote:

> Hi,
>
> On Wed, Jun 1, 2016 at 1:06 PM Mark Abraham <mark.j.abraham at gmail.com>
> wrote:
>
>> So I've fixed them, and after two months Teemu found time to look at the
>> straightforward fix + tests. After a further month, nobody else has had
>> time to look. Meanwhile, it isn't worth me continuing development to
>> support such a feature until I see that somebody cares about it.
>>
>
> I agree that the delay in getting things reviewed is sometimes very long.
> But the above is a bit one-sided. It is true that it took me two months to
> find time to review stuff (because of various reasons, including other
> Gromacs work and general lack of free time), and there's not much more to
> say about that. But once I did provide comments, the next month the patch
> just sat there in Gerrit, waiting for some of my comments to be addressed.
> It might very well be that no one else looked at it, but if I had not been
> the one to post comments and had looked at it, I would have just ignored it
> because there were still unaddressed comments from someone else. After the
> findings were addressed, it took all of a four days to get it merged.
>
> The responsiveness in reviewing stuff goes both ways. I have currently
> some 75 changes in the "incoming reviews" category, which is something like
> 50% of all open changes in Gerrit! And quite a few of those are really old,
> where either the change doesn't even pass Jenkins, where there have been
> comments from reviewers and then no action whatsoever afterwards, or that
> has been long since been obsoleted by some other change. This is a huge
> demotivating factor for reviewing stuff, not to mention that it is a waste
> of time scanning through the same list over and over again to see whether
> there would be a change somewhere in that 50+ changes (or in the 150+ open
> changes) that could be usefully reviewed...
>
> Best regards,
> Teemu
> --
> Gromacs Developers mailing list
>
> * Please search the archive at
> http://www.gromacs.org/Support/Mailing_Lists/GMX-developers_List before
> posting!
>
> * Can't post? Read http://www.gromacs.org/Support/Mailing_Lists
>
> * For (un)subscribe requests visit
> https://maillist.sys.kth.se/mailman/listinfo/gromacs.org_gmx-developers
> or send a mail to gmx-developers-request at gromacs.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20160602/96aab321/attachment.html>