[gmx-developers] Some thoughts about new trajectory analisys api

[Teemu Murtola]

Hi,

On Tue, Feb 5, 2013 at 9:07 PM, David van der Spoel <spoel at xray.bmc.uu.se>wrote:

> On 2013-02-05 01:17, Roland Schulz wrote:
>
>> On Mon, Feb 4, 2013 at 3:41 PM, Schulz, Roland <rschulz3 at utk.edu
>> <mailto:rschulz3 at utk.edu>> wrote:
>>     Why would you want to copy sel? I thought it would be sufficient to
>>     copy x. angle.cpp has a copy_pos.
>>
>
The copy_pos() in angle.cpp originally had a lot more functionality than it
currently has, so it isn't really a good example. ;) The original point was
to encapsulate different input formats for the selections (either tuples of
n atoms in a single selection, or n separate selections), but that is now
all handled by the "merge" selection keyword so the tool is much simpler,
and would not really require that copy (it does not modify the vectors,
just puts them into a separate array for readability).

It might be nice to have that in
>>     Selection, e.g.:
>>     void copy_x(int first, int last, rvec x[]) const;
>>
>> Actually that suggestion is very non-std like. It combines access to the
>> complete range of coordinates ,with the algorithm (copy), and thus is
>> less flexible because these two things can't be combined differently.
>> So it might be better to provide: "iterator xBegin()" and "iterator
>> xEnd()". That would allow to copy using:
>> "std::copy(sel.xBegin(),sel.**xEnd(),out)".
>>
>
I think the best way is to provide a method like ConstArrayRef<rvec>
positionArray() const, which automatically gives those iterators as well.
Haven't tried this, but this should compile. But using std::copy() on a
container of rvecs doesn't work, because rvec is not copyable or
assignable. This part still needs work, but is a more general problem than
just in the analysis framework.

But I'm not sure we discussed how much we want to make use of iterators.
>> Our modified version changes that mpp uses iterators and we think it got
>> better/more flexible by doing that. I think Teemu tried to avoid them.
>>
>
You are probably referring to http://redmine.gromacs.org/issues/856.
Overall, iterators are nice (and several classes in the code provide
iterator access or take iterators as parameters), but for that particular
case would have been tricky to implement (and would still not have strictly
fulfilled the strict requirements for an iterator). Also, I'm using
iterators extensively for looping over containers.

>
> Iterators are not mentioned at all on http://www.gromacs.org/index.**
> php?title=Developer_Zone/**Programming_Guide/Allowed_C%**2B%2B_Features<http://www.gromacs.org/index.php?title=Developer_Zone/Programming_Guide/Allowed_C%2B%2B_Features>


It does mention that "do use STL". ;) And STL uses iterators extensively. I
see no reason to limit using iterators defined in STL, nor implementing
simple iterators for our custom classes. But creating very complex
iterators may not classify as "simple C++".

Teemu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20130205/e5d72964/attachment.html>