<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 5, 2013 at 9:07 PM, David van der Spoel <span dir="ltr"><<a href="mailto:spoel@xray.bmc.uu.se" target="_blank">spoel@xray.bmc.uu.se</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On 2013-02-05 01:17, Roland Schulz wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On Mon, Feb 4, 2013 at 3:41 PM, Schulz, Roland <<a href="mailto:rschulz3@utk.edu" target="_blank">rschulz3@utk.edu</a><br>
</div><div class="im">
<mailto:<a href="mailto:rschulz3@utk.edu" target="_blank">rschulz3@utk.edu</a>>> wrote:<br> Why would you want to copy sel? I thought it would be sufficient to<br></div><div><div class="h5">
copy x. angle.cpp has a copy_pos.</div></div></blockquote></blockquote><div><br></div><div style>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).</div>
<div style><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5"> It might be nice to have that in<br>
Selection, e.g.:<br>
void copy_x(int first, int last, rvec x[]) const;<br>
<br>
Actually that suggestion is very non-std like. It combines access to the<br>
complete range of coordinates ,with the algorithm (copy), and thus is<br>
less flexible because these two things can't be combined differently.<br>
So it might be better to provide: "iterator xBegin()" and "iterator<br>
xEnd()". That would allow to copy using:<br>
"std::copy(sel.xBegin(),sel.<u></u>xEnd(),out)".<br></div></div></blockquote></blockquote><div> </div><div style>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div class="h5">
But I'm not sure we discussed how much we want to make use of iterators.<br>
Our modified version changes that mpp uses iterators and we think it got<br>
better/more flexible by doing that. I think Teemu tried to avoid them.<br></div></div></blockquote></blockquote><div><br></div><div style>You are probably referring to <a href="http://redmine.gromacs.org/issues/856">http://redmine.gromacs.org/issues/856</a>. 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.</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Iterators are not mentioned at all on <a href="http://www.gromacs.org/index.php?title=Developer_Zone/Programming_Guide/Allowed_C%2B%2B_Features" target="_blank">http://www.gromacs.org/index.<u></u>php?title=Developer_Zone/<u></u>Programming_Guide/Allowed_C%<u></u>2B%2B_Features</a></blockquote>
<div><br></div><div style>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++".</div>
<div style><br></div><div style>Teemu</div></div></div></div>