bugGNU Octave - Bugs: bug #39838, str2double ('') gives [](0x1)...

 
 

bug #39838: str2double ('') gives [](0x1) instead of NaN

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Wed 21 Aug 2013 01:14:49 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Philip Nienhuis 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

Wed 11 Sep 2013 03:36:35 PM UTC, comment #17: 

I pushed the posted patch here (http://hg.savannah.gnu.org/hgweb/octave/rev/3baf8e7ec3dd).  Closing report.

Rik <rik5>
Group administrator
Fri 06 Sep 2013 06:14:10 PM UTC, comment #16: 

If your goal of octave is to get people off of matlab and onto GNU/FOSS/whatever, then you're either following two strategies 1.) making a language that for all intents and purposes is a matlab replacement, or 2.) making something that is dramatically technically superior.   It seems like the approach of octave is 1,  whereas projects like python, R, Julia are taking the approach of 2. 

Creating a language that is "kind of" a matlab replacement, with most of the technical limitations of such a language, and which follows some of the idiosyncrasies for compatibility but not others simply because matlab is illogical, doesn't seem like a good approach.

Anonymous
Fri 06 Sep 2013 06:10:00 PM UTC, comment #15: 

Slightly revised patch attached

(file #29037)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 06 Sep 2013 12:39:57 PM UTC, comment #14: 

@Ben:
Seems I beat you to it :-), changeset attached.
All tests pass, including the ones you added. I've edited my revised help text a little more and swapped it in.

I added your name + email address in the patch header; it is mostly your work not so much mine. Only in the log on the hg repo your name will be shown, I'll see that your name won't appear in the contributors list (yet?). Would you agree to this?


(file #29029)

Philip Nienhuis <philipnienhuis>
Group Member
Thu 05 Sep 2013 08:58:32 PM UTC, comment #13: 

WRT Matlab's behavior on multi-row strings:

  • Matlab's behavior is undocumented.  no mention is made in the str2double documentation of what the behavior will be
  • Other similar functions (e.g. hex2num) take a similar interpretation of converting one number per row.
  • Matlab's behavior is illogical, as it converts down columns, rather than across rows.  In other words, it follows the data order in memory (completely ignoring the array size) rather than following standard (english at least) language conventions.
  • Matlab's behavior is inconsistent between versions.  Philip's example of


str2double (['1'; ' '; '2'])

errors out on his version of matlab, and simply returns NaN in the version of matlab I have installed.

I'll see about a changeset at some point if no one else gets to it first.

Ben Kurtz <bkurtz>
Tue 03 Sep 2013 08:10:25 PM UTC, comment #12: 

Ben K, as to comment #9:
There's a nice write-up on how to prepare changesets (by Jordi himself):

http://jordi.inversethought.com/blog/how-to-write-a-patch-for-octave/

If you do make a changeset, could you also please include the revised help text I uploaded in comment #6 and check that it fits the new str2double behavior?
I don't mind making a cs myself and pushing it but this week my evenings turn out to have been reserved for furniture repair jobs at home, my own and my daughter's sport clubs, and some parent gathering at her school. No way to escape those ;-)
Maybe Friday I'll have some time for str2double. Feel free to beat me to it, at your discretion.

Of course I'd also like to try & test your patch but that can be done any time before pushing.

@Jordi:
Sorry but I don't follow.
I think we (Ben and I) do follow Matlab as far as reasonable.
Ben -like me- prefers option A1 in comment #9 which AFAIU does basically the same as Matlab.
We'll deviate from ML in that Octave's str2double continues to accept string arrays as input, but with some unavoidable gotchas that I tried to outline in the revised help text.

Anyway I didn't write it yet but this thread confirms my opinion that Matlab isn't stupid - there is valid reasoning for e.g., its str2double to not accept char array input. But that's no reason for Octave to blindly follow. Let's at least make Octave's docs more clear than ML's.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 03 Sep 2013 04:56:54 PM UTC, comment #11: 

The use of "stupid" is not too helpful.  Making
decisions based on the "intelligence" of Matlab
is not appropriate.  Better to use technical terms,
like: incorrect, subject to misinterpretation,
incomplete, ...

Ben did say "probably incorrect" which is a sound
argument.

Michael Godfrey <godfrey>
Group Member
Tue 03 Sep 2013 04:49:38 PM UTC, comment #10: 

Ben, upload the output of "hg export".

As to not following Matlab because Matlab is stupid, boy, working on Octave would be a lot easier if we could just afford to declare Matlab stupid all the time and not follow it. Are you sure that this exactly what we have to do now?

Jordi GutiƩrrez Hermoso <jordigh>
Group Member
Tue 03 Sep 2013 05:37:49 AM UTC, comment #9: 

Wrt logistics, I'll probably pass on being added in the contributors file for now.  If I contribute more significantly later, then I'll see about getting myself added.  To prepare a changeset, would it work for me to make a commit in my local hg repository and then upload the output of

hg log -p -l 1

or do I need to be able to push to the main octave repository?  (Also: although I'm fairly comfortable with git, I haven't used hg much, so if there are better commands let me know).

Wrt the fix itself:

  • I think we both agree that matlab's implementation for multi-row strings is undocumented, confusing, and probably incorrect.  As such, I think we should not follow matlab's behavior in this case, but instead continue with octave's previous behavior of handling an N-row string the same way we would an Nx1 cell array containing those rows as the cell contents.  I think we're agreed on this, but if I'm mis-interpreting, please let me know!
  • Empty rows are tricky.  I see several cases.  Blank rows (all spaces) can exist in octave char matrix data, since rows that are too short are padded with spaces.  I think it is unambiguous that (as long as we are converting rows), we should convert each blank row to a NaN, including if the matrix consists entirely of spaces.  Empty rows (i.e. rows of zero length) cannot exist in an MxN matrix with N!=0, as all ways of trying to add one result in it either being ignored, or it being padded with N spaces.  The question is how to deal with a char matrix that is entirely empty (i.e. one for which isempty() returns true; i.e. one of the dimensions is zero).  The way I've currently implemented is to interpret any empty string as being equivalent to "the empty string" which converts to a scalar NaN (let's call this option A1).  This is a slight deviation from "treating it as if each row were a cell in a cell array", which would convert a 0xN char array to a 0x1 NaN and an Mx0 char array to an Mx1 NaN (let's call this option B1).  Unfortunately, that is the behavior in the current release of octave, and is what you filed the bug report against, and is inconsistent with matlab on basically all inputs.


As I think I've indicated, I prefer option A1, but I'm not sure which one you think is better.  Perhaps we could return just a scalar NaN, but issue a warning for Mx0 or 0xN char arrays when M or N !=0 (call this one option A2) saying that the user is probably not doing what they want?

Ben Kurtz <bkurtz>
Sun 01 Sep 2013 10:40:18 AM UTC, comment #8: 

Thanks, I'll try tonight or Tuesday (my boxes are occupied with other builds now). I'll also try to add a few more tests.

Your name is not in contributors.in (http://hg.savannah.gnu.org/hgweb/octave/file/95412dcfa707/doc/interpreter/contributors.in) - would you like to be added? (few people object). How would you like to be attributed in the final changeset (see http://hg.savannah.gnu.org/hgweb/octave, pick a log entry and see how it looks like in the "author:" line), OR:
Better yet (for me ;-) ) can you prepare a changeset rather than a diff patch, including the revised help text?


As to char array input:
- Again, note that Matlab simply doesn't mention char array input for str2double. That was already so for r2007a (oldest version I have tried). If absurd, it must be that it doesn't  immediately warn when trying to process a char array, but only when it gets really unparsable. See below for r2007a output:


>> str2double (['1'; ''; '2'])
ans =
    12
% No mention of erroneous input!
>> str2double (['1'; ' '; '2'])
??? Error using ==> strrep
Input strings must have one row.
Error in ==> str2double at 44
    s = strrep(s, ',','');


(...where it shows in passing that ML deletes commas while processing strings. BTW an interesting outcome for bug #34713)

- Yes, char arrays are normally padded to equal string element (row) length (by Octave, internally). My point -again- is that str2double's input can be generated by other procedures or scripts [1], even outside Octave, that are unaware of this, so we simply have to be prepared for those procedures to generate empty elements in char arrays. If not technically (while processing) then at least 'verbally' (by warning users that output may be smaller in size than expected).

[1] I cannot easily come up with an example of how to do that in practice, but I have no doubts that it is possible.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 01 Sep 2013 04:03:34 AM UTC, comment #7: 

And here's my (belated, sorry) patch implementing the changes I suggested:

  • char data with either rows or columns equal to zero returns a scalar NaN.
  • non-char data of any size also converts to scalar NaN.  It seemed silly to match the size of non-char data, since you have no idea what kind of data it's trying to represent (e.g. str2double(double("123.5")) likely shouldn't be nan(1,5)).


This leaves us slightly different from matlab, in that matlab treats an MxN char matrix as a single 1x(M*N) string, reading down the columns, which is simply an absurd interpretation of a char array.  I think octave's existing implementation in that case - namely, to convert each row separately - makes good sense, so I left that exactly as is, except of course for the empty cases.  Note that there aren't cases where some rows are empty (length 0) and some rows have valid data, because all rows must have the same length.  Storing multi-row string data in a single char seems like a bad idea in general for this reason, but at least we can try to work with people on that.

(file #28971)

Ben Kurtz <bkurtz>
Tue 27 Aug 2013 08:59:15 PM UTC, comment #6: 

OK here's a first draft of str2double's revised help text (as a swap-in for current text), assuming we keep Octave's extension (compared to Matlab) of character matrices being accepted as input; I've added a few words on deleting corresponding empty rows.

It is also assumed that we (try to) return a double/complex array rather than a scalar NaN for character input in case a/any non-empty row couldn't be converted.


(file #28944)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Aug 2013 08:09:50 PM UTC, comment #5: 

As to 3x1 => 2x1 char array, you're quite right, ML does the same (never noticed, I very rarely work with char arrays).
My point was that char type input arrays for str2double can be "machine-generated", and anything beyond str2double in the processing order can be unpleasantly surprised by silently adapted dimensions. But I do think cell arrays are more suited for this kind of thing; the dimensions for char arrays are adapted as soon as one char array element is empty -str2double isn't even required-, cell arrays are immune to this.

Before you make your patch, do note that ML's docs for str2double (r2013b prerelease) do not (explicitly) mention char arrays, only cell arrays. (For str2num, char arrays are explicitly mentioned, but not cell arrays; same for e.g., hex2num.)
In this respect it may be enlightening to see what ML's str2double does with char arrays:

>> str2double (['1'; ''; '3'])

ans =

    13

>> str2double (['1 + 0.5i'; '';])

ans =
   1.0000 + 0.5000i

>> str2double (['1 + 0.5i'; ''; '3 + 2.1i'])

ans =

   NaN


.... so Matlab's str2double even treats a char array as one long string (IOW it doesn't recognize char arrays); again quite different from Octave. Maybe that's why it returns a scalar NaN for an "unparsable" char array.

My offer to adapt str2double's help text still stands. I'll await your patch, I might have time next Tuesday evening.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Aug 2013 08:07:26 AM UTC, comment #4: 

A couple of things.  First, the call to

    str2double (['2'; ''; "1"])

is actually somewhat misleading.  At first glance, it looks like you might be passing a 3x1 char into str2double, but in fact, if you evaluate the argument by itself, it turns out that the empty string is zero by zero, and so is ignored in the vertcat operation.

It seems like probably the correct behavior is just to match what Matlab does here?  It looks to me like matlab's procedure is well described as:
1. if it's a cell, return an array of the same size, with NaN for any element that's not a string or doesn't convert correctly.
2. if it's a char array:
 2a. If any dimension is zero, return a scalar NaN.
 2b. Otherwise return a column vector with one result per row of the char array.  Any position that couldn't be converted should be NaN, obviously.
3. For any other input, regardless of size, return a scalar NaN.

This primarily differs from octave's current implementation on item 2a and item 3, where octave currently prefers to match the size of non-string input and the number of rows of empty strings.

I'll put together a quick patch tomorrow.

Anonymous
Sat 24 Aug 2013 10:30:52 AM UTC, comment #3: 

Hmm..., in the last part of my comment it should read:

"after we have decided about ('') input"

Philip Nienhuis <philipnienhuis>
Group Member
Sat 24 Aug 2013 10:29:08 AM UTC, comment #2: 

Although IMO it's a bit close to the edge I agree with the str2double({}) outcome. At least that case makes sense.

For the other cases I tend to simply follow the help text's message that "all output that str2double cannot parse" (after having detected input to be no cell array) "gives a NaN".

In particular as to str2double (''), as shown below, str2double's current behavior as regards arrays of strings can be deceptive:

>> str2double (['2'; ''; "1"])
ans =

   2
   1


...so in such a case str2double doesn't return an array equal in size as the input array. I suppose that is (or should be) the very rationale that NaNs are always needed to reflect any empty string input: to prevent str2double from silently "post-resizing" its class double output.
In case of cell arrays str2double behaves correctly:

>> str2double ({'2'; ''; '1'})
ans =

    2.0000e+00
           NaN
    1.0000e+00


Anyway, after we have decided about ({}) input, at least the st2double help text could be amended to better clarify its behavior (I'll do that then). Something like:

1. Is it a cell array (optionally with empty cells?)
..yes-> return equally sized array (see 3.)
..no->  go to 2
2. Is it a string or array of strings?
..yes-> return equally sized array (see 3.)
..no->  return NaN
3. For all unparsable individual cells str2double returns a NaN

Philip Nienhuis <philipnienhuis>
Group Member
Fri 23 Aug 2013 05:59:55 PM UTC, comment #1: 

The behavior that `str2double({})` ==> `[]` is actually correct.  The rationale being that you have given a cell array containing no strings to convert, and that you should therefore get a list of no numbers in response.

Arguably, the `str2double('')` case is also documented to return an empty value,  since rows of a matrix are documented as being interpreted separately, and the empty string is 0x0 so has no rows.  Note that `str2double(char(zeros(n,0)))` gives `nan(n,1)` in octave.  However, as you observe, this differs from MATLAB's behavior, which is to return a single NaN for any empty string (i.e. anything with a zero dimension).

MATLAB's behavior also differs on any input that is not of char or cell type; octave returns `nan(size(input))`, while matlab returns `nan(1)`.

Anonymous
Wed 21 Aug 2013 01:14:49 PM UTC, original submission:  

According to the Octave docs, str2double(S) "...returns NaN for elements of S which cannot be converted".

However, in Octave 3.7.5 (and I also saw this in Octave 3.4.3):

debug> str2double('')
ans = [](0x1)
debug> str2double([])
ans = [](0x0)
debug> str2double(' ')
ans = NaN
debug> str2double('a')
ans = NaN
debug> str2double({})
ans = [](0x0)
debug> str2double({''})
ans = NaN
debug> str2double({[]})
ans = NaN
debug>


...so str2double doesn't always behave as the docs say it does.
(Note I hit this while debugging Matlab scripts at work)

Matlab r2012a gives: (empty lines removed)

>> str2double('')
ans =
   NaN
>> str2double([])
ans =
   NaN
>> str2double(' ')
ans =
   NaN
>> str2double('a')
ans =
   NaN
>> str2double({})
ans =
     []
>> str2double({''})
ans =
   NaN
>> str2double({[]})
ans =
   NaN
>>


...so also Matlab sometimes gets confused, e.g., str2double({}) (Matlab docs claim it should return NaN there, in wording similar to Octave)

Philip Nienhuis <philipnienhuis>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #29029:  str2double_39838.cs added by philipnienhuis (3KiB - text/x-csharp)
file #28971:  str2double_empties.patch added by bkurtz (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by jordigh (Posted a comment)
  • -email is unavailable- added by bkurtz (Updated the item)
  • -email is unavailable- added by philipnienhuis (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-09-11 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2013-09-06 philipnienhuis Attached File- Added str2double_39838-v2.cs, #29037
    2013-09-06 philipnienhuis Attached File- Added str2double_39838.cs, #29029
        StatusNone Ready For Test
    2013-09-01 bkurtz Attached File- Added str2double_empties.patch, #28971
    2013-08-27 philipnienhuis Attached File- Added str2double_texinfo_header_bug#39838_v1.txt, #28944

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code