bugGNU Octave - Bugs: bug #50619, textscan weird behaviour when...

 
 

bug #50619: textscan weird behaviour when reading a csv

Submitter:  Andrea <rackbox>
Submitted:  Thu 23 Mar 2017 02:33:04 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 22 Oct 2017 07:47:41 PM UTC, comment #16: 

I can confirm that after applying the two patches in bug 52116 this bug seems fixed.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 27 Sep 2017 09:54:53 AM UTC, comment #15: 

I believe I have found this bug.  See the patch attached to the following bug report:

https://savannah.gnu.org/bugs/index.php?52116#comment10

I tested with the original example here, and it seems to work.

Dan Sebald <sebald>
Sat 25 Mar 2017 11:27:21 PM UTC, comment #14: 

Good detective work, Dan. Obviously it looked like some buffer size issue but which buffer and where it is to be found ...

Answer to comment #11 Q1:
< "endofline", "" > is perfectly legal syntax and can be useful when reading from strings (i.e., not a text file, or a file not standard organized along lines like e.g., XML or html)

AFAIU from my textscan.m/strread.m work from years gone by, endofline is whitespace but whitespace is a default delimiter unless "%s" (character string) format specifiers come into play, in which case endofline is moved to the delimiter collection. Or something like that ... the (=TMW's) logic is almost untractable.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 25 Mar 2017 11:26:36 PM UTC, comment #13: 

To the original poster, the following mods work for me (add new line to the delimiters and increase the buffer size):


provaPath = 'testfile.csv'
file = fopen(provaPath);
formatSpec = '%s %s %s %s %s %s %s';
[col_headers, pos1] = textscan(file, formatSpec, 1, 'Delimiter', ";\n", 'BufSize', 4096);
formatSpec = '%f %f %f %f %f %f %f';
[logLine, pos2] = textscan(file, formatSpec, 1, 'Delimiter', ";\n", 'BufSize', 4096);
fclose(file);


It's not a bug fix, but if you keep your string lengths well below 4096, the above should work.

Dan Sebald <sebald>
Sat 25 Mar 2017 10:38:58 PM UTC, comment #12: 

Here's the comment for the delimited_stream object:


  // Delimited stream, optimized to read strings of characters separated
  // by single-character delimiters.
  //
  // The reason behind this class is that octstream doesn't provide
  // seek/tell, but the opportunity has been taken to optimise for the
  // textscan workload.
  //
  // The function reads chunks into a 4kiB buffer, and marks where the
  // last delimiter occurs.  Reads up to this delimiter can be fast.
  // After that last delimiter, the remaining text is moved to the front
  // of the buffer and the buffer is refilled.  This also allows cheap
  // seek and tell operations within a "fast read" block.


I take it that using the std::stream routines are slow when reading one character at a time, so this class brings in a chunk of data to remain resident and have faster access.  I don't know the value of a non-absolute seek and tell that only refer to some given buffer.

Also, what if there is no "last delimiter" in the buffer because the length of the string is greater than the length of the buffer?

Another issue, given chunks of data is the goal, is that the existing code doesn't go about it in a very efficient way.  For example, here is the code that reads a string:


        std::string vv ("        ");      // initial buffer.  Grows as needed
        switch (fmt.type)
          {
          case 's':
            scan_string (is, fmt, vv);
            break;


For every field read, there is a new standard string "on the stack".  Now, "on the stack" probably means that the actual character data isn't on the stack, but the object's reference is (for otherwise the stack could easily overflow if the string is very long).  But the point is, for every field there's a fresh std::string that has to be grown.  Instead, why not keep a std::string as part of the delimited_stream object?  One that grows as large as the largest field it ever sees, and when the delimiter stream is deleted, so is the memory associated with the std::string.

Because, looking at scan_string:


        for (i = 0; i < width; i++)
          {
            if (i+1 > val.length ())
              val = val + val + ' ';      // grow even if empty
            int ch = is.get ();
            if (is_delim (ch) || ch == std::istream::traits_type::eof ())
              {
                is.putback (ch);
                break;
              }
            else
              val[i] = ch;
          }


every time through the loop one has to check if the length of std::string is big enough.  Why not use a fast character scanning function to determine the length to the next delimiter first, then expand the std::string, then copy data?

There are a lot of FIXMEs in this code, probably because of a not-fully-planned buffer scheme.

Dan Sebald <sebald>
Sat 25 Mar 2017 06:46:26 PM UTC, comment #11: 

Yes, that is a duplicate.

OK, I'm a little further along in the thought process, and I see now why this strange behavior and the formula for "he [deg]" works, but not "heading [deg]" does not work.

The delimiter buffer size is 80.  Count 80 characters out of the below buffer:


time [s];lat [deg];lon [deg];x [m];y [m];speed [m/s];heading [deg]
5.2500000000000;44.000000000000000;10.000000000000000;0.000000000000000;0.000000000000000;0.000000000000000;44.998483574087999


and that points 4 characters BEFORE the semicolon of the second line.  Snip six characters from the first line and then that first semicolon of the second line is AFTER 80 characters.  So, the interaction of that delimited_stream with the end of its buffer and putting stuff back into buffer is where the error lies.  And that is why when I put ";\n" in for the delimiter characters, the fields come out right, but the "5.25" is dropped--the delimiter_stream buffer has grabbed a new chunk of data from the std::stream, so what the delimiter stream is attempting to put back, is lost. (? That's the theory anyway.)

By that contorted thinking, lengthening the delimiter_stream buffer from 80 to 100 should fix this particular problem when I use ";\n" delimiters...


    // Next, choose a buffer size to avoid reading too much, or too often.
    octave_idx_type buf_size = 4096;
    if (buffer_size)
      buf_size = buffer_size;
    else if (ntimes > 0)
      {
        // Avoid overflow of 80*ntimes...
//        buf_size = std::min (buf_size, std::max (ntimes, 80 * ntimes));
        buf_size = std::min (buf_size, std::max (ntimes, 100 * ntimes));
        buf_size = std::max (buf_size, ntimes);
      }
    // Finally, create the stream.
    delimited_stream is (isp,
                         (delim_table.empty () ? whitespace + "\r\n" : delims),
                         max_lookahead, buf_size);


And that does, in fact, work:


octave:8> logLine
logLine =
{
  [1,1] =  5.2500
  [1,2] =  44
  [1,3] =  10
  [1,4] = 0
  [1,5] = 0
  [1,6] = 0
  [1,7] =  44.998
}


but of course this isn't a general fix, because the first line could be any length.

OK, so there are two things wrong in the delimiter_stream code

1) The EOL character is not automatically included as a delimiter.  I guess it should be in all cases, correct?  That is, there isn't some form of syntax for textscan() for which the user can specify EOL is not a delimiter?

2) The buffer doesn't behave correctly at the end, most likely because valuable characters are dropped when delimiter_stream buffer does a refresh_buf():


    void field_done (void)
    {
      if (idx >= last)
        refresh_buf ();
    }


Dan Sebald <sebald>
Sat 25 Mar 2017 03:48:51 PM UTC, comment #10: 

bug #50485 may be related

Philip Nienhuis <philipnienhuis>
Group Member
Sat 25 Mar 2017 03:44:10 PM UTC, comment #9: 

The EOL ("endofline") char / char sequence should automatically be included in the list of delimiters.

Be sure to not forget the \r char.
FYI textscan has an option <"endofline", <EOL char> >


Philip Nienhuis <philipnienhuis>
Group Member
Sat 25 Mar 2017 10:06:22 AM UTC, comment #8: 

I've tracked this down a bit, so I'm just writing some notes here for reference:

I printed out the "is.tellg()" for:


  void
  textscan::scan_string (delimited_stream& is, const textscan_format_elt& fmt,
                         std::string& val) const
  {
    if (delim_list.is_empty ())
      {
        unsigned int i = 0;
        unsigned int width = fmt.width;

fprintf(stderr, "width=%d\n", width);
        for (i = 0; i < width; i++)
          {
fprintf(stderr,"+%d",i);
            if (i+1 > val.length ())
              val = val + val + ' ';      // grow even if empty
            int ch = is.get ();
            if (is_delim (ch) || ch == std::istream::traits_type::eof ())
              {
fprintf(stderr, "address = %u\n", is.tellg());
                is.putback (ch);
                break;
              }
            else
              val[i] = ch;
          }
        val = val.substr (0, i);          // trim pre-allocation
      }
    else  // Cell array of multi-character delimiters


Here's the result for the test case:


+0+1+2+3+4+5+6+7+8address = 7867337
+0+1+2+3+4+5+6+7+8+9address = 7867347
+0+1+2+3+4+5+6+7+8+9address = 7867357
+0+1+2+3+4+5address = 7867363
+0+1+2+3+4+5address = 7867369
+0+1+2+3+4+5+6+7+8+9+10+11address = 7867381
+0+1+2+3+4+5+6+7+8+9+10+11+12+13address = 7867343


What this is telling me is that the pointer advances as expected with the is.get().  That is, the count of +1, etc. is the number of characters added to the pointer's previous value to get (hopefully) the next pointer address.  Except until the last field, the fourteen character "heading [deg]".  In that case the pointer makes some odd jump, going backward (!), as we'd expect 7867381 + 14 = 7867395.

This stream:


    delimited_stream is (isp,
                         (delim_table.empty () ? whitespace + "\r\n" : delims),
                         max_lookahead, buf_size);


isn't behaving nicely.  The max_lookahead is 3, and the buf_size is 80.  (I recall somewhere else there being a buffer size of 4096...but don't take that as being of some significance, as I don't quite understand the implication of buf_size.)

I can see what is wrong.  See the delims passed into this delimited stream?  Later in testing the ch = is.get() character with is_delim(ch), it's those delims (a C++ std::string) that are looked for.  Going into that is() instantiation is only ";".  So this delimited_stream doesn't recognize the new-line character as a delimiter.  It's just another character, so the delimiter stream keeps reading until hitting another ";" character.  There must be some odd relationship between line length and buf_size that causes the pointer to advance to some strange place in the next line for the next textscan().  Note: I think that even though the col_headers looks to be reading the "header [deg]" properly, I think it's not and somehow the new-line character-plus (i.e., "\n5.2500000000000") is dropped somewhere along the way when converted to cell-string.

So, as a little test, let's try putting ";\n" in for the delimiters in the test code, i.e., textscan(file, formatSpec, 1, 'Delimiter', ";\n"):


+0+1+2+3+4+5+6+7+8address = 7866201
+0+1+2+3+4+5+6+7+8+9address = 7866211
+0+1+2+3+4+5+6+7+8+9address = 7866221
+0+1+2+3+4+5address = 7866227
+0+1+2+3+4+5address = 7866233
+0+1+2+3+4+5+6+7+8+9+10+11address = 7866245
+0+1+2+3+4+5+6+7+8+9+10+11+12+13address = 7866259


OK, now things look proper, i.e., 7866245 + 14 = 7866259.  Unfortunately, the result still isn't quite correct:


octave:16> logLine
logLine =
{
  [1,1] = 0
  [1,2] =  44
  [1,3] =  10
  [1,4] = 0
  [1,5] = 0
  [1,6] = 0
  [1,7] =  44.998
}


Better!  But the first entry isn't 5.25.  Again, some strange interaction with the new-line character and placing it back into the stream, maybe?

That's where I am.  On the trail, I think, but only close so far.

Dan Sebald <sebald>
Fri 24 Mar 2017 09:23:38 PM UTC, comment #7: 

The file does look perfectly suited to dlmread with a skip of the first line.

Does this work?


data = dlmread ("file.csv", ";", 1, 0);



Rik <rik5>
Group administrator
Fri 24 Mar 2017 12:23:47 PM UTC, comment #6: 

100,000's of lines is no big deal. 10e5 * 100 chars/line ~ 10 MB, that's still fairly tiny considering that char arrays can be 2 GB (with 64-bit Octave).
As soon as you hit 1 GB (a factor of 100 away) you should become a little wary as data are copied over internally into potentially much larger data objects. But do use 64-bit Octave.

Chances are that textscan() won't be fixed very soon. So you need a workaround.
I'd advise trying with dlmread, csvread or csv2cell anyway; in top (*nix) or Task Manager (Windows) you can follow how much memory is consumed. You can read big datafiles in big chunks at a time.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 24 Mar 2017 08:29:20 AM UTC, comment #5: 

It is a log file whose production I can't control, I have to use it as-is. It can contain hundreds of thousands of lines.

Andrea <rackbox>
Fri 24 Mar 2017 08:25:34 AM UTC, comment #4: 

Define "huge".

I do see that it spoils huge amounts of disk space because of possibly unneeded output precision, i.e. 16-17 digits.
Cutting down there would make the file maybe half as huge  :-)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 24 Mar 2017 08:14:46 AM UTC, comment #3: 

I was using textscan because the file I want to read is huge, and I need to read it line by line...

Andrea <rackbox>
Thu 23 Mar 2017 11:43:22 PM UTC, comment #2: 

Confirmed on Windows 7 with Octave-4.3.0+
An intriguing bug.
I checked with Matlab r2017a but that does not portray this bug.

AFAICS, by starting with "h" for the last field on line 1 and adding characters, trouble starts as soon as the "[" (in fact, any character) is added after the trailing space after "heading".  Then textscan starts reading in the middle of the "44.0000000000000" field (correctly at the faulty pos1 position) and gets out of sync, evidenced by the NaN (= missing value for empty field).
Adding a <'whitespace', ''> arg in the textscan call makes no difference.
Textcan correctly reads the last field on line 1 in all cases. So it is the file pointer computation that gets confused.

BTW
As a workaround you can also try to read the csv-file using csv2cell in the io package (I just tried). As of io-2.4.6 (now silently updated to 2.4.7) csv2cell accepts a spreadsheet-style address argument, see "help csv2cell".

>> C = csv2cell ('bug50619.csv', ';')
C =
{
  [1,1] = time [s]
  [2,1] =  5.2500
  [1,2] = lat [deg]
  [2,2] =  44
  [1,3] = lon [deg]
  [2,3] =  10
  [1,4] = x [m]
  [2,4] = 0
  [1,5] = y [m]
  [2,5] = 0
  [1,6] = speed [m/s]
  [2,6] = 0
  [1,7] = heading [deg]
  [2,7] =  44.998
}


Philip Nienhuis <philipnienhuis>
Group Member
Thu 23 Mar 2017 05:09:35 PM UTC, comment #1: 

I've confirmed this behavior in linux with the development version.  The pertinent code resides in libinterp/corefun/oct-stream.cc, probably textscan::do_scan().

The only other detail I see is that ";he [deg]" also works, but not ";hea [deg]".  With ";he [deg]", the total length of the first line is 64 characters, including the carriage-return character.  I presume what is happening is the first textscan() is searching for the next field after the first group of reads and advances the file pointer.  But the REPEAT field is present, "1", so the algorithm should stop after that seventh string field.

Dan Sebald <sebald>
Thu 23 Mar 2017 02:33:04 PM UTC, original submission:  

If I process this .csv file


time [s];lat [deg];lon [deg];x [m];y [m];speed [m/s];heading [deg]
5.2500000000000;44.000000000000000;10.000000000000000;0.000000000000000;0.000000000000000;0.000000000000000;44.998483574087999


with the following code


file = fopen(provaPath);
formatSpec = '%s %s %s %s %s %s %s';
[col_headers, pos1] = textscan(file, formatSpec, 1, 'Delimiter', ';');

formatSpec = '%f %f %f %f %f %f %f';
[logLine, pos2] = textscan(file, formatSpec, 1, 'Delimiter', ';');


the output of the second textscan call is wrong. In particular, the first element (which should be 5.25) is 0. In this case the value of 'pos1' is 96 (why?).

If I change the .csv file substituting the word "heading" with "h" and leave everything else unchanged, the output is correct. In this case the value of 'pos1' is 62 (correct).

This behaviour is not reproducible in Matlab 2010, which produces the correct output in both cases.


Andrea <rackbox>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by rackbox (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-10-26 rik5 StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
    2017-10-22 philipnienhuis StatusConfirmed Patch Reviewed
    2017-10-21 rik5 Dependencies- Depends on bugs #52116
    2017-04-04 mmuetzel Dependencies- bugs #50717 is dependent
    2017-03-23 philipnienhuis StatusNone Confirmed
        Release4.2.1 dev
        Operating SystemMicrosoft Windows Any

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code