bugGNU Octave - Bugs: bug #49536, many functions that expect a...

 
 

bug #49536: many functions that expect a "string" silently truncate char matrix arguments (octave_value::string_value)

Submitted by:  Mike Miller <mtmiller>
Submitted on:  Sat 05 Nov 2016 12:53:07 AM UTC  
 
Category: LibrariesSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Other
Status: ConfirmedAssigned to: None
Originator Name: Open/Closed: Open
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 28 Nov 2016 10:21:46 PM UTC, comment #9:

I think the fix in comment #8 is worthwhile. @Mike: Do you want to go ahead and push it?

Rik <rik5>
Project Administrator
Wed 23 Nov 2016 08:32:21 PM UTC, comment #8:

The warning comes from the following backtrace

I am now using this change

and can now work with the GUI and run the full test suite in the GUI with no warnings.

Needs some polish and a warning ID, but I think this is a good first start to draw a line.

Mike Miller <mtmiller>
Project Administrator
Wed 23 Nov 2016 08:12:12 PM UTC, comment #7:

I made the following change in my local build

The good news: I can run `make check RUN_OCTAVE_OPTIONS=-cli` and I get no new test failures or warnings printed on the console or in the log file.

The bad news: I can't run `make check` or run the test suite inside of the GUI. The test suite errors out completely when it gets to libinterp/corefcn/error.cc. The octave_qt_link appears to be doing something whenever a value is in the workspace that always evaluates a char matrix as a string. I haven't tracked that down yet. Even something as simple as `x = ["one"; "two"];` starts printing the warning message after every prompt until the value is cleared from the workspace.

Mike Miller <mtmiller>
Project Administrator
Tue 22 Nov 2016 02:55:34 PM UTC, comment #6:

As a start, I agree with Colin because it at least doesn't silently discard data. Longer term, I think we should identify those functions that are different from Matlab and consider individually whether they should be fixed for compatibility.

John W. Eaton <jwe>
Project Administrator
Tue 22 Nov 2016 06:34:33 AM UTC, comment #5:

I was surprised by this so I think its significant.

I'd vote for 2 (warning with current behaviour). Seems like path of least resistance.

Colin Macdonald <cbm>
Mon 07 Nov 2016 08:02:28 PM UTC, comment #4:

Thanks for the testing Ceral.

Philip ­- it could be argued that Octave behaves slightly worse here, because it is silently throwing away data. At least with the few examples I've shown here, Matlab either gives the entire array (yes, it is flattened in column major order, but it's complete), or throws an error saying that the argument must be a vector.

I would propose that Octave could do one of three things to make this better

1. Always return the entire array, squeezed into a vector following the normal column-major ordering.

2. Issue a warning while doing what it does today.

3. Throw an error when code tries to extract a string_value from an argument that has more than 1 non-singleton dimension.

Any of these changes can be made in just one place, in the octave_char_matrix_str::string_value method, and affect any function that tries to get a simple string from one of its arguments.

Mike Miller <mtmiller>
Project Administrator
Sat 05 Nov 2016 12:32:14 PM UTC, comment #3:

Is this really a problem?
AFAICS char matrices aren't often used. cellstr arrays are much more practical.

Matlab has the same issue and even behaves slightly worse (which in a way is reassuring :-) )

Could transposing a char array before feeding it to string functions help?
For sprintf with-a-format-string, help and strrep it works to the extent that a 1xN vector is returned but in the right "order"; the other functions you mention are still oblivious.

Philip Nienhuis <philipnienhuis>
Project Member
Sat 05 Nov 2016 09:19:06 AM UTC, comment #2:

Matlab:

% added spaces to make strings the same length

>> s = [' this '; ' is a '; ' char '; 'matrix']


s =

this
is a
char
matrix

>> sprintf (s)


ans =

mticahshti arsari x

>> sprintf ('%s', s)


ans =

mticahshti arsari x

>> error (s)

mticahshti arsari x

>> help (s)

Warning: The following error was caught while executing 'helpUtils.helpProcess' class destructor:
Input must be a row vector of characters.

> In help (line 45)
>> struct (s, 0)

Error using struct
Field name must be a string vector.

>> system (s);

Error using system
Argument must contain a string.

>> strrep (s, 'a', 'b')

Error using strrep
Input strings must have one row.

Ceral Paquet <octavebugs>
Sat 05 Nov 2016 09:13:33 AM UTC, comment #1:

Matlab:

>> s = ["this"; "is a"; "char"; "matrix"]

s = ["this"; "is a"; "char"; "matrix"]

Error: The input character is not valid in MATLAB statements or expressions.

>> s = ['this'; 'is a'; 'char'; 'matrix']

Dimensions of matrices being concatenated are not consistent.

Ceral Paquet <octavebugs>
Sat 05 Nov 2016 12:53:07 AM UTC, original submission:

Octave has many functions that operate on a char array with the concept of a "string", even though char arrays may be two- or higher-dimensional. Octave currently silently truncates char arrays when given to some functions that are expecting a string. Other functions flatten the char array into a vector (in column-major order). Some motivating examples:

The guilty function is octave_value::string_value. The code is labeled with a FIXME where it deliberately pulls out the first row of a 2-dimensional char array, so that seems like a prime candidate for discussion and resolution.

The list of functions affected is probably very long, but maybe we can gather together a short list and get a Matlab compatibility check as a starting point.

Mike Miller <mtmiller>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by cbm (Posted a comment)
  • -unavailable- added by octavebugs (Posted a comment)
  • -unavailable- added by mtmiller (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 28 Nov 2016 10:23:24 PM UTCrik5Dependencies-=>bugs #49696 is dependent
    Mon 28 Nov 2016 10:21:46 PM UTCrik5StatusPostponed=>Confirmed
    Sat 05 Nov 2016 12:56:23 AM UTCmtmillerSummarymany functions that expect a "string" silently truncate char matrix arguments (octave_value::string_vector)=>many functions that expect a "string" silently truncate char matrix arguments (octave_value::string_value)

    Back to the top


    Powered by Savane 3.1-cleanup1