<div dir="ltr">Hi Robin,<div><br></div><div>Thanks for this. You said you were directed to report bugs here, but the preferred procedure would now be to open a Github issue and I thought I had changed most references to that by now (of course, documentation in releases cannot be changed retroactively). If you would point out where the reference to this list was, I can hopefully clean that up. In fact, this list will be closing down soon in favor of Github Discussions. There is a message from me to that effect waiting for moderator approval.</div><div><br></div><div>In any case, if you would not mind, can you open an issue on this? <a href="https://github.com/coin-or/CoinUtils/issues/new">https://github.com/coin-or/CoinUtils/issues/new</a></div><div><br></div><div>Cheers,</div><div><br></div><div>Ted</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 7, 2021 at 9:42 AM Robin Whittle <<a href="mailto:rw@firstpr.com.au">rw@firstpr.com.au</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Though there hasn't been a message on this list for 2 years . . . we are<br>
instructed to report bugs to the relevant list.<br>
<br>
<br>
In CoinUtils 2.11.2 and 2.11.3 - and I guess earlier versions, but I<br>
haven't checked - the code in CoinIndexedVector.cpp for<br>
CoinIndexedVector::reserve() differs from that from version 2.10.11.<br>
See the attached image to see how the two differ.<br>
<br>
I am using 2.10.11 and have no plans to use the later code, but I did<br>
notice one or two things which look wrong in these later versions.<br>
<br>
In 2.10.11, reserve() takes no action if the requested capacity == the<br>
current capacity.<br>
<br>
if (n < capacity_) {...}<br>
else if (n > capacity_)<br>
<br>
This looks correct to me. In the new versions, it takes one kind of<br>
action (line 541 in 2.11.3, with some added parentheses for clarity):<br>
<br>
if ((n + nPlus) < capacity_)<br>
<br>
and the other if (line 556):<br>
<br>
else if (n > capacity_)<br>
<br>
I suspect that line 556 should be:<br>
<br>
else if ((n + nPlus) > capacity_)<br>
<br>
which would replicate the prior no action arrangement if n == capacity_.<br>
<br>
Functionally, the code may still work OK, but it looks wrong to me and<br>
there are no comments to explain why it is so.<br>
<br>
nPlus seems to be set according to the size of integers and I guess it<br>
represents the extra data items in the object beyond the two arrays:<br>
<br>
int nElements_;<br>
int capacity_;<br>
int offset_;<br>
bool packedMode_;<br>
<br>
Also, given line 541, it seems odd that line 585 remains:<br>
<br>
capacity_ = n;<br>
<br>
The intended functionality of the various parts of this function could<br>
have been clearly described with suitable comments. Then I wouldn't<br>
have to guess about what it is supposed to do. In the absence of such<br>
comments I can't know for sure what it supposed to do, but I wanted to<br>
point out what seems to me like potential problems.<br>
<br>
Robin<br>
_______________________________________________<br>
CoinUtils mailing list<br>
<a href="mailto:CoinUtils@list.coin-or.org" target="_blank">CoinUtils@list.coin-or.org</a><br>
<a href="https://list.coin-or.org/mailman/listinfo/coinutils" rel="noreferrer" target="_blank">https://list.coin-or.org/mailman/listinfo/coinutils</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><span style="font-size:12.8px">Dr. Ted Ralphs</span><br style="font-size:12.8px"><span style="font-size:12.8px">Professor, Lehigh University</span><br style="font-size:12.8px"><span style="font-size:12.8px">(610) 628-1280</span><br style="font-size:12.8px"><span style="font-size:12.8px">ted 'at' lehigh 'dot' edu</span><br style="font-size:12.8px"><a href="http://coral.ie.lehigh.edu/~ted" style="font-size:12.8px" target="_blank">coral.ie.lehigh.edu/~ted</a><br></div></div></div>