OK, the attached version with makes use of extract_num() is much easier to comprehend.  extract_num() and a couple helper routines are moved to lo-utils.cc unaltered.  Because extract_num() doesn't handle upper case I and J, I used std::transform to preprocess the   convert the whole string with a simply little helper routine.  Slightly less efficient, but so be it for now, unless str2double eventually handles I and J as well.

(file #44988)

Not feeling up to the challenge, but there is a function extract_num in str2double.cc which probably does the right thing.

I forgot to fully recompile, so the NaN+Infj case is cleared up with a rebuild.  However,

1.5,1+j,4

fails still.  On the other hand, the following works

1.5,1+1j,4

octave:4> importdata ('tst_1_1j.txt', ',')
ans =

   1.5000 + 0.0000i   1.0000 + 1.0000i   4.0000 + 0.0000i

So, we have an issue still with dlmread interpretting a standalone i/j/I/J.  I'll look into this, but I wonder if perhaps we should be doing some code reuse here with some small routine in the core parser rather than redoing if/then statement.

OK, I see the reasoning behind the "NaN + Infj", I guess.  It is being thought of as

(Inf,0) * (0,j) = Inf*0 + Inf*j + 0*0 - 0*1
                = NaN + Infj + 0 - 0
                = NaN + Infj

Not sure I completely agree with that, but that's fine.  I mean, we could make the argument that

-1 * Inf = (-1,0) * (Inf,0)
         = -1*Inf + -1*0 j + Inf*0 j - 0*0
         = -Inf + NaN j

whatever.

However, I then tried some things.  The attached change to importdata.m BIST works.  On the other hand, reading "NaN+Infj" doesn't seem to work.  I changed Rik's example file to be:

1.5,NaN+Infj,4

and I see that the NaN+Infj is not recognized

octave:1> dlmread ('tst_nan_infj.txt', ',')
ans =

   1.50000   0.00000   4.00000

octave:2> importdata ('tst_nan_infj.txt', ',')
ans =

  scalar structure containing the fields:

    data =

       1.5000       NA   4.0000

    textdata =
    {
      [1,1] = NaN+Infj
    }

Do we need to further enhance the if-else block in dlmread.cc?

(file #44984)

I pushed a change to get rid of std::tolower and just check for the four individual letters (https://hg.savannah.gnu.org/hgweb/octave/rev/11f63a33732e).

@John: Is that enough to fix this report?

@Dan: I think it's probably fine to check explicitly for 'i','I','j','J' rather than using tolower.  However, we can't move the test for non-finite values up in the if/else if tree because it then fails to read text like 'Infj' to create a complex value with Infinite imaginary part.  Attaching a simple test file for that which you can test with
 

dlmread ('tst_infj.txt', ',')

(file #44982)

With the patch from comment #50, I get a consistent result of:

d = dlmread('c:\octave\num.txt', '\t', 0,0, "emptyvalue", NA)

d =

     3.1000        Inf         NA
       -Inf        NaN   128.0000

Note the diffence of Nan vs NA, hoever running test importdata now passes.

But:

>> d = importdata ('c:\octave\num.txt', '\t')

d =

     3.1000        Inf         NA
       -Inf        NaN   128.0000

>>

Nah, that character after "Inf" must be a x09.  Clearly, std::isalpha (next_char) thinks that 255 is alphanumeric for John's system.  Depends on the locale, I guess:

http://www.cplusplus.com/reference/cctype/isalpha/

The way I understand this routine is that some error bit during the double conversion such that peek() returns an EOF value:

http://www.cplusplus.com/reference/istream/istream/peek/
http://www.cplusplus.com/reference/ios/ios/rdstate/

Perhaps it would be better to move that condition to the top if-statement.  Also, looking at this, perhaps we should just avoid using the tolower() routine.  Of interest is only the i,j,I,J letters and it is only two extra machine cycles to if I and J are present.  Whereas, with tolower() there needs to be a call/jump/return via a library routine, and that library routine is going to have to check some ranges to make sure that the character in question can be converted to some other ASCII.  Why do all that just to account for a couple extra characters?

I'm attaching a changeset that might solve the problem by avoiding tolower().  Please give it a try.  If it doesn't work, then we'll have to dig deeper.

(file #44972)

"I wrote a quick C++ program and tolower(-2) returned 254 which is weird."  -2 in two's complement is the same bit representation as 254 unsigned.  Bitwise, arithmetic is pretty much the same, just how one treats the sign bit:

Unsigned
--------
255  FF
254  FE
  ...
128  F0
127  7F
  ...
0    00

Signed
------
127  EF
  ...
0    00
-1   FF
-2   FE
  ...
-128 F0

I think you are right about the lolower() not being defined.  Perhaps the ASCII test needs to be done prior to the tolower() call to confirm it is in proper range before treating as ASCII.

I have answers, but I can at least locate where the problem is occurring.  It is in this code

          double x = octave_read_double (tmp_stream);
          if (tmp_stream)
            {
              if (tmp_stream.eof ())
                {
                  if (iscmplx)
                    cdata(i,j++) = x;
                  else
                    rdata(i,j++) = x;
                }
              else
                {
                  int next_char = std::tolower (tmp_stream.peek ());
                  if (next_char == 'i' || next_char == 'j')

When it works, the tmp_stream.peek() call returns EOF which is -1.  When it fails, the next char is 255.  One change to make would be to break out the peek and the tolower call.  Calling tolower with EOF returns EOF, but using any other negative value is undefined.  I wrote a quick C++ program and tolower(-2) returned 254 which is weird.  So, perhaps modify

int next_char = std::tolower (tmp_stream.peek ());

to

int next_char0 = tmp_stream.peek ();
printf ("next_char0: %d\n", next_char0);
int next_char = std::tolower (next_char0);
printf ("next_char: %d\n", next_char);

Following the code, octave_read_double eventually calls octave_read_fp_value in liboctave/utils/lo-utils.cc.

// Read a double value.  Discard any sign on NaN and NA.

template <typename T>
double
octave_read_fp_value (std::istream& is)

The trouble may also be here or in read_inf_nan_na().

Looking at the issue in dlmread.

I added some print statements to dlmready, but havent had a change to take a real look to see if it tells anything usefull yet:

diff -r f7ef179096ee libinterp/corefcn/dlmread.cc
--- a/libinterp/corefcn/dlmread.cc      Thu Sep 06 11:01:14 2018 -0400
+++ b/libinterp/corefcn/dlmread.cc      Mon Sep 10 10:24:11 2018 -0400
@@ -310,6 +310,8 @@

   std::istringstream tmp_stream;

+  printf("dlmread cmax=%d rmax=%d, rcnt=%d, r0=%d, r1=%d\n", (int)cmax, (int)rmax, (int)rcnt, (int)r0, (int)r1);
+
   // Read the data one field at a time, growing the data matrix as needed.
   while (getline (*input, line))
     {
@@ -317,7 +319,7 @@
       if ((! sep_is_wspace || auto_sep_is_wspace)
           && line.find_first_not_of (" \t") == std::string::npos)
         continue;
-
+printf("getline: '%s'\n", line.c_str());
       // Infer separator from file if delimiter is blank.
       if (sep.empty ())
         {
@@ -396,7 +398,7 @@
         pos1 = line.find_first_not_of (" \t");  // Skip leading whitespace.
       else
         pos1 = 0;
-
+printf("doloop pos1=%d rmax=%d cmax=%d\n", (int)pos1, (int)rmax, (int)cmax);
       do
         {
           octave_quit ();
@@ -430,7 +432,7 @@
               else
                 rdata.resize (rmax, cmax, empty_value);
             }
-
+printf("v='%s'\n", str.c_str());
           tmp_stream.str (str);
           tmp_stream.clear ();

@@ -439,6 +441,7 @@
             {
               if (tmp_stream.eof ())
                 {
+                       printf("set %lf\n", x);
                   if (iscmplx)
                     cdata(i,j++) = x;
                   else
@@ -447,6 +450,7 @@
               else
                 {
                   int next_char = std::tolower (tmp_stream.peek ());
+                       printf("not end - next=%d\n", next_char);
                   if (next_char == 'i' || next_char == 'j')
                     {
                       // Process pure imaginary numbers.
@@ -494,6 +498,7 @@
             {
               // octave_read_double() parsing failed
               j++;  // Leave data initialized to empty_value
+             printf("read double failed\n");
             }

           pos1 = pos2 + 1;

The on running:

d = dlmread('num.txt', '\t', 0,0, "emptyvalue", NA)

(where num.txt is file #44946)



Running the command before the test suite:

>> d = dlmread('num.txt', '\t', 0,0, "emptyvalue", NA)

dlmread cmax=0 rmax=32, rcnt=0, r0=0, r1=-2
getline: '3.1   Inf     NA'
doloop pos1=0 rmax=32 cmax=2
v='3.1'
set 3.100000
v='Inf'
not end - next=-1
v='NA'
set nan
getline: '-Inf  NaN     128'
doloop pos1=0 rmax=32 cmax=3
v='-Inf'
not end - next=-1
v='NaN'
not end - next=-1
v='128'
set 128.000000
d =

     3.1000        Inf         NA
       -Inf        NaN   128.0000

>>

Running it after the test suite was run:

>> d = dlmread('num.txt', '\t', 0,0, "emptyvalue", NA)

dlmread cmax=0 rmax=32, rcnt=0, r0=0, r1=-2
getline: '3.1   Inf     NA'
doloop pos1=0 rmax=32 cmax=2
v='3.1'
set 3.100000
v='Inf'
not end - next=255
v='NA'
set nan
getline: '-Inf  NaN     128'
doloop pos1=0 rmax=32 cmax=3
v='-Inf'
not end - next=255
v='NaN'
not end - next=255
v='128'
set 128.000000
d =

     3.1000         NA         NA
         NA         NA   128.0000

>>

I pushed a patch for the one problem of being unable to read NA with CRLF line ending (https://hg.savannah.gnu.org/hgweb/octave/rev/75f7e8728e17).

I think that John might be experiencing a slightly different issue than Rik.  See Comment #34, Comment #30 and Comment #4.  The importdata.m script does tweak some post-dlmread things, but I can't find anywhere in the code where it re-interprets the Inf and NaN text fields.  importdata.m just moves those fields into the textdata return value, so I can't see how anything would correct the NA coming from dlmread.  The difference may be in the compiler/C-library used to build on your two systems and how it handles that <istream> >> double extraction operator:

http://www.cplusplus.com/reference/istream/istream/operator%3E%3E/

The issue Rik sees in Windows (and probably John too if only it would get past the dlmread data problem) is something I can replicate on Linux if I force the same scenario.

With the \r\n, true change, I still get the same BISt error after running the test suite.

OK, thanks.  I see that importdata.m does not go back and reinterpret the Inf and NaN text hunks.  So, it is all up to dlmread.cc and that octave_read_double() of Comment #24.  I'm wondering why it is then that Windows is going into some kind of mode in which it can no longer read those.

I tried searching for, say, oct-stream that might do the Inf/NaN work, but it appears dlmread.cc is using pretty much the standard C++99 (or higher) steams for which (I think) the Inf and NaN should be interpreted by the standard library code.  Is the file being opened in an incorrect mode?  E.g., binary rather than text?

It's rather difficult finding where this is all done in the source code without some nice unique keywords to grep for.  Anyway, let's approach this by confirming a couple things.  If you are able to recompile, I'm attaching a patch that will dump what goes in and comes out of octave_read_double().  The modification is in a CC file so shouldn't cause a long recompile of the project.  Please run that and show the result before/after the other BIST tests.

For comparison, here's the kind of output I'm seeing on Linux:

octave:28> A = [3.1 Inf NA; -Inf NaN 128];
octave:29> fn  = tempname ();
octave:30> fid = fopen (fn, "w");
octave:31> fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
octave:32> fclose (fid);
octave:33> %dlmread (fn, '\t', 0, 0, "emptyvalue", NA)
octave:33> [a,d,h] = importdata (fn, '\t');
STR: "3.1"  CONVERTED AS: 3.1
STR: "Inf"  CONVERTED AS: inf
STR: "NA"  CONVERTED AS: nan
STR: "-Inf"  CONVERTED AS: -inf
STR: "NaN"  CONVERTED AS: nan
STR: "128"  CONVERTED AS: 128
file_content =
{
  [1,1] = 3.1        Inf        NA
  [1,2] = -Inf        NaN        128
}

octave:34> unlink (fn);
octave:35> assert (a, A);
octave:36> assert (d, "\t");
octave:37> assert (h, 0);
octave:38>
octave:38> A = [3.1 Inf NA; -Inf NaN 128];
octave:39> fn  = tempname ();
octave:40> fid = fopen (fn, "w");
octave:41> fputs (fid, "3.1\tInf\tNA\r\n-Inf\tNaN\t128");
octave:42> fclose (fid);
octave:43> %dlmread (fn, '\t', 0, 0, "emptyvalue", NA)
octave:43> [a,d,h] = importdata (fn, '\t');
STR: "3.1"  CONVERTED AS: 3.1
STR: "Inf"  CONVERTED AS: inf
"  CONVERTED AS: nan
STR: "-Inf"  CONVERTED AS: -inf
STR: "NaN"  CONVERTED AS: nan
STR: "128"  CONVERTED AS: 128
file_content =
{
  [1,1] = 3.1        Inf        NA
  [1,2] =
  [1,3] = -Inf        NaN        128
}

octave:44> unlink (fn);
octave:45> assert (a, A);
octave:46> assert (d, "\t");
octave:47> assert (h, 0);

In the above the "missing" STR: is actually the '\r' character moving the line pointer back to the start.  (It's kind of unfortunate that the BIST field before the \n is of the NaN variety, but not that important.)

(file #44955)

Running initially (before test suite) with the ostrsplit:

>>  A = [3.1 Inf NA; -Inf NaN 128];
>>  fn  = tempname ();
>>  fid = fopen (fn, "w");
>>  fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
>>  fclose (fid);
>>  [a,d,h] = importdata (fn, '\t');
>> unlink (fn);
>> a
a =

     3.1000        Inf         NA
       -Inf        NaN   128.0000

>> assert(A, a)
>> d
d =
>> h
h = 0
>>

Running after the run of test suite:

>>  A = [3.1 Inf NA; -Inf NaN 128];
>>  fn  = tempname ();
>>  fid = fopen (fn, "w");
>>  fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
>>  fclose (fid);
>>  [a,d,h] = importdata (fn, '\t');
>>  unlink (fn);
>> a
a =

  scalar structure containing the fields:

    data =

         3.1000         NA         NA
             NA         NA   128.0000

    textdata =
    {
      [1,1] = Inf
      [2,1] = -Inf
      [3,1] = NaN
    }

>> assert(a, A)
error: ASSERT errors for:  assert (a,A)

  Location  |  Observed  |  Expected  |  Reason
     .          O(1x1)       E(2x3)      Dimensions don't match
>>

"The NaN is not processed as expected" meaning that dlmread, although handling +/-Inf, doesn't handle the NaN, so it is the second pass that Rik pointed out which catches the NaN.  In other words, neither of those two are working quite right.

from comment #20, its happening in the dlmread - return more NA then there should be in the failing tests vs non failing test

I forget, these are stream objects:

http://www.cplusplus.com/reference/fstream/ifstream/close/

"
Note that any open file is automatically closed when the ifstream object is destroyed.
"

That theory is nixed.

"..., but I don't see an associated "close()" on that file when all done."

Meaning that it could be the dlmread() isn't properly closing the file so the file isn't rewound before "fileread (fname)" of

    file_content = ostrsplit (fileread (fname), "\n");

tries reading in the exact same ASCII that dlmread() processed.

The dlmread() routine opens a file as follows:

      input_file.open (ascii_fname.c_str (), std::ios::in);

where the second argument is mode.  Now, this is a bit more complex than file mode options that I've seen.  They are discussed here:

http://www.cplusplus.com/reference/ios/ios_base/openmode/

Notice how none of these are used directly.  Instead, the documentation gives some kind of derived class ios_base::in, which also is not the same as std::ios::in.  I don't know if the settings are fixed based upon the way std::ios::in is declared, or perhaps based upon the way std::ios::in instance was opened, or what.

From there, it isn't too complicated.  The routine just looks at each character to determine if the ascii makes sense in terms of complex number format, etc. and when it determines it should read a double does:

template <typename T>
T
octave_read_value (std::istream& is)
{
  T retval;
  is >> retval;
  return retval;
}

[snip]

// The next four functions are provided for backward compatibility.
inline double
octave_read_double (std::istream& is)
{
  return octave_read_value<double> (is);
}

Of course, there can be a lot hidden in that ">>" operator depending on how the stream is opened.

@Dan comment #12:
I think you're right. "test histcounts" in a 1:1000 for loop got me some FAILs.

As it isn't my code I'll first await if the OP of patch #8801 will fix it.

continuing from comment 18, using a premade file #44946 and running it before the test suite:


data = dlmread ('test.txt', '\t', 0, 0, "emptyvalue", NA);

returns

3.1000 Inf NA
-Inf NA 128.0000

And after running the test suite:

3.1000 NA NA
NA NA 128.0000


So although it does fail with text files, its not the issue occurring here (although maybe related)

Comparing a good run vs a fail run, the first time I see if differ in the outputs is at:

output.data = dlmread (fname, delimiter, header_rows, header_cols,
                           "emptyvalue", NA);

At that point output.data is

fail run:


     3.1000         NA         NA
         NA         NA   128.0000

pass run:

     3.1000         Inf        NA
       -Inf         NA   128.0000

Of course I may have to take that comment back somewhat - changing it to fid = fopen (fn, "wt"), it fails even without having to run the test suite.

File size of an extra character bigger at that point though, which was not happening on the previously failing files

Just a thought, the creation of the file is done with binary mode, despite the fact that this is a text file.  The code is

fn  = tempname ();
fid = fopen (fn, "w");
fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
fclose (fid);

According to the documentation for fopen,

     Append a "t" to the mode string to open the file in text mode or a
     "b" to open in binary mode.  On Windows systems, text mode reading
     and writing automatically converts linefeeds to the appropriate
     line end character for the system (carriage-return linefeed on
     Windows).  The default when no mode is specified is binary.

There is an embedded newline '\n' in there which on Linux, in binary mode, is fine because that is the line-ending character.  On Windows, however, this probably fails to create a 2-line file.  Maybe try altering the BIST test to

fid = fopen (fn, "wt");

and see if things work.

@Philip, I see the code.  Without looking at it too much, my guess is that these are two different issues.  Consider reposting the finding at the histcounts report:

https://savannah.gnu.org/patch/?8801

The thing is, that fail involves random data and it could be that something like the following floor() function isn't quite right with regards to numerical approach:

+  if (equidistant)
+    bin = 1 + floor ((x - min_val) / binwidths(1)); % compute bin indices in O(Nx)
+  else
+    bin = lookup (edges, x); % compute bin indices in O(Nx*log(nbins)), all x < edges(1) will be zero
+  endif
+  bin(x > edges(end)) = 0;                % set too large entries to zero
+  idx = bin(bin != 0);                    % remove all out-of-scope entries
+  n = accumarray (idx, 1, [nbins+1, 1])'; % cout number of identical indices to get the histogram, in O(Nx)
+  n = [n(1:end-2), n(end-1)+n(end)];      % last bin should also contain right edge
+  % xx = edges(1:end-1) + binwidths/2;    % place bin centers half-way between the 'edges'

That is, when the floating point calculation of the bins and normalization is done, it could be that the max_val (or min value) goes epsilon beyond the edge border.  Don't know, as I didn't investigate much.  Could try simply running the histcounts BIST a hundred times with nothing before or after and see if it fails somewhere with a different random set of data.

The file is the same in both cases, so it isn't the input file that is the source of the problem.

None of these data reading routines is documented well with regard to the output, in this case A.  It's strange that it is creating the expected matrix as a sub-element of a structure, then also some extra bit of information.  It appears to me that the extra bit of information is a bunch of strings that the routine cannot translate.  That textdata, i.e.,

    textdata =
    {
      [1,1] = Inf
      [2,1] = -Inf
      [3,1] = NaN
    }

suggests that importdata, or dlmread, or something else is in a mode in which "Inf" etc. cannot be properly read as float/double values.  Perhaps one of the previous texts changed some flag somewhere that controls that.

I find it odd that textdata doesn't have some means of associating the text field with its (possibly multiple) position in the matrix.  Since that information is lost, it's not of much use after the import to the user in terms of reconstructing what happened.

That was failing file #44945

@Dan: dlmread has BIST tests for it in dlmread.cc.

@John: Can you modify the BIST test to remove the "unlink (fn)" statement and instead use

printf ("filename: %s\n", fn);

When it fails, can you go find the actual text file and upload it to this report?

>> A = [3.1 Inf NA; -Inf NaN 128];
>>  fn  = tempname ();
>>  fid = fopen (fn, "w");
>>  fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
>>  fclose (fid);
>>  [a,d,h] = importdata (fn, '\t');
>>  unlink (fn);
>> A
A =

     3.1000        Inf         NA
       -Inf        NaN   128.0000

>> a
a =

  scalar structure containing the fields:

    data =

         3.1000         NA         NA
             NA         NA   128.0000

    textdata =
    {
      [1,1] = Inf
      [2,1] = -Inf
      [3,1] = NaN
    }


>> d
d =
>> h
h = 0
>>

dlmread() is now the heart of importdata(), for ASCII.  I don't see and BIST for dlmread(), there probably should be.

Could you print out the values of a, d and h?  Just copy the lines of code (after the main check) without running the test specifically.  I'm curious what 1x1 result a is.  Is it all one big string of text perhaps?  I can't understand how the algorithm would get stuck reading 3.1 and nothing else beyond that first tab character.

Running a current version of dev octave in Windows I get a test failure for importdata which doesnt seem to be happening in linux

It failed when running _run_test_suite_, and then afterwards will fail the same running the test separately

>> test importdata
***** test
 ## Exceptional values (Inf, NaN, NA)
 A = [3.1 Inf NA; -Inf NaN 128];
 fn  = tempname ();
 fid = fopen (fn, "w");
 fputs (fid, "3.1\tInf\tNA\n-Inf\tNaN\t128");
 fclose (fid);
 [a,d,h] = importdata (fn, '\t');
 unlink (fn);
 assert (a, A);
 assert (d, "\t");
 assert (h, 0);
!!!!! test failed
ASSERT errors for:  assert (a,A)

  Location  |  Observed  |  Expected  |  Reason
     .          O(1x1)       E(2x3)      Dimensions don't match

Unfortunately, if I run test importdata before testsuite, it passes ok.