[Octave-bug-tracker] [bug #50102] dlmread crashing the interpreter on Cygwin

[Dan Sebald]

Follow-up Comment #22, bug #50102 (project octave):

OK, improvement.  That test against 1 is sort of redundant in that the apriori
condition on rmax is/was known.  The double expanding is the way to go.  I
like that.  I see that you've chosen to not double expand the columns because
that is estimated at the beginning line and typically won't keep expanding
throughout the rest of the file (although it is allowed to).  

However, I think the dual tests on rmax and cmax simultaneously leads to this
not always being what the comment indicates.  If I understand correctly,


          if (r > rmax || c > cmax)
            {


is meant to allow adjustments for column size as reading goes forward.  Let's
say starting at row 35 is the following:

1,34,64
2,45,83
...
35,58,60
36,26,99,-3
37,53,78,-5
...

Because c expands by one at row 36, the above condition is going to test true.
 So, at that point r is 36, and then there is an automatic readjustment even
though it was the column size that changed:


              // Use resize_and_fill for the case of unequal length rows.
              // Keep rmax a power of 2.
              rmax = std::max (2*(r-1), rmax);
              cmax = std::max (c, cmax);


so rmax becomes 2*(r-1) = 70, not a power of two (if the power of two is
preferred).  There are two ways of approaching this.  Minimizing the potential
number of reshapes per pass:


          if (r > rmax || c > cmax)
            {
              if (r > rmax)
                  rmax = std::max (2*(r-1), rmax);
              cmax = std::max (c, cmax);
[reshape]


A second is minimizing a little extra code within the test:


          if (r > rmax)
            {
              rmax = std::max (2*(r-1), rmax);
[reshape]
            }
          if (c > cmax)
            {
              cmax = c;
[reshape]
            }


The benefit of one over the other is dependent on the statistical nature of
the data and even then doesn't make much difference.  So, it boils down to
preference.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?50102>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/