[gmx-developers] Virtual site live review tomorrow

[Joe Jordan]

I plan to attend, but may be a few minutes late.

On Wed, Feb 10, 2021 at 8:01 PM Pascal Merz <Pascal.Merz at colorado.edu>
wrote:

> Hi Erik,
>
> I see that I formulated my email a bit imprecisely. I mentioned in the
> telco earlier today that I will spend some time before the live review to
> see if there are things that can be separated into separate commits (with
> future bisection in mind). But I’m convinced that it makes sense to review
> them together. Whether this ends up being several MRs or one MR with
> non-squashed commits is up for discussion.
>
> Also, I am certainly willing to take responsibility for the change. I am
> reasonably confident to do so since before this change, we had almost no
> test coverage of virtual sites (except for a few regression tests) - at the
> level that some virtual site types were introduced, but could not be run
> because the rest of the infrastructure was never updated to allow for
> additional types. The change introduces unit and end-to-end tests, while
> not breaking any of the regression tests that were present. The current
> change also fixes several issues: The virtual site positions were slightly
> wrong in some cases, the virtual velocities ranged from totally wrong to
> slightly wrong to “correct by chance”.
>
> I am fully aware that there might be additional issues, but I am also
> confident that I am leaving the code in significantly better shape than it
> was before.
>
> Best,
> Pascal
>
>
>
> On Feb 10, 2021, at 10:32 AM, Erik Lindahl <erik.lindahl at gmail.com> wrote:
>
> Hi,
>
> Unfortunately I have zero time the next two weeks.
>
> My main worry with these large changes is not the first-time review (which
> indeed can be done with a live review), but that we've had several such
> large changes cause subtle bugs with potentially devastating results for
> other simulations - and apart from the fact that it's take a while to
> notice them, it's also taken a while to find when it's something that was
> part of a large change.
>
> In particular the ~700 changed lines in vsite.cpp appears to be a mix of
> general cleanup and a couple of different changes - so I don't quite see
> how it's *impossible* to separate those into something more manageable.
>
> So, just remember that it's not merely a matter of believing the code path
> you focus on works, but that the ~700 lines won't have side effects on any
> other code paths, and that large changes going in also means writing a
> blank check to rapidly look into any future bugs that are bisected to that
> change ;-)
>
> Cheers,
>
> Erik
>
>
> On Wed, Feb 10, 2021 at 6:12 PM Pascal Merz <Pascal.Merz at colorado.edu>
> wrote:
>
>> Hi all,
>>
>>
>> The virtual sites MR at
>> https://gitlab.com/gromacs/gromacs/-/merge_requests/979 is rather large,
>> but cannot easily be split into smaller MRs. It should be reasonably easy
>> to review with a bit of guidance, however, since the large majority of line
>> changes are documentation, tests, and repetitive changes.
>>
>> To that end, Mark and I will have a live review session of the change on
>> Feb 11, 9am MST / 5pm CET. If someone else would like to jump on to help
>> the review, I would very much appreciate it! I think that we should be able
>> to keep it around 30 minutes, but it all depends on how many issues we find
>> :) We’ll meet at https://cuboulder.zoom.us/j/94868986659.
>>
>> Thank you,
>> Pascal
>>
>> --
>> 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.
>
>
>
> --
> Erik Lindahl <erik.lindahl at dbb.su.se>
> Professor of Biophysics, Dept. Biochemistry & Biophysics, Stockholm
> University
> Science for Life Laboratory, Box 1031, 17121 Solna, Sweden
> --
> 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.
>
>
> --
> 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/20210211/0605dba7/attachment.html>