bugGNU Octave - Bugs: bug #52542, Weird behaviour of vertical...

 
 

bug #52542: Weird behaviour of vertical concatenation with empty char array

Submitter:  Luis Mendo <lmendo>
Submitted:  Mon 27 Nov 2017 11:57:39 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Luis Mendo Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 30 Nov 2017 06:13:54 PM UTC, comment #8: 

After some test results on the Octave Maintainer's list I committed the patch to the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/1212ffc13d24).

This will be a part of the 4.2.2 bug fix release expected by the end of the year.

Rik <rik5>
Group administrator
Tue 28 Nov 2017 11:18:26 PM UTC, comment #7: 

Of course, this turns out to be way, way more complicated.  It appears the actual issue is in the parse tree and the parser's concatenation of string constants.  I've attached a patch that works for me, but jwe should probably review as this is not my area.  It seems like there were points in the parsing of


double([char(ones(1,0)); 'A'])


where the dimension vector (dv) of the first element was checked for emptiness with


dv.zero_by_zero ()


However, there are other ways of having an empty matrix.  It is possible that the code should be reviewed and the construct above replaced with the one below.


dv.any_zero ()


For the attached patch, there is another slight issue in that it doesn't flag a warning when the dimensions are different as is done for other data types.


octave:1> double([char(ones(0,3)); 'A'])
ans =  65
octave:2> double([ones(0,3); 'A'])
error: vertical dimensions mismatch (0x3 vs 1x1)




(file #42512)

Rik <rik5>
Group administrator
Tue 28 Nov 2017 06:19:31 AM UTC, comment #6: 

convert_to_str is implemented by each octave_value type by the function convert_to_str_internal.  For a real matrix like ones() the code should be in libinterp/octave-value/ov-re-mat.cc.  The function convert_to_str internal in that file begins with


  octave_value retval;
  dim_vector dv = dims ();
  octave_idx_type nel = dv.numel ();

  charNDArray chm (dv);

  bool warned = false;

  for (octave_idx_type i = 0; i < nel; i++)


This looks alright, unless the constructor for charNDArray is somehow not doing the right thing.

Rik <rik5>
Group administrator
Tue 28 Nov 2017 06:12:51 AM UTC, comment #5: 

char() is located in libinterp/corefcn/strfcns.cc.  The opening lines are


  if (nargin == 0)
    retval = "";
  else if (nargin == 1)
    retval = args(0).convert_to_str (true, true,
                                     args(0).is_dq_string () ? '"' : '\'');
  else
    {


That suggests convert_to_str() has a problem, but it also means there is a third case where multiple arguments are passed to the char function.  Indeed, this fails as well in a different manner.


double ([char(ones(0,4), ones(4,0)); 'A'])
ans =

   65
   32
   32
   32
   32
   32



Rik <rik5>
Group administrator
Tue 28 Nov 2017 06:07:11 AM UTC, comment #4: 

It is definitely the char() function call which is creating the problem.  I can use string_fill_char() to set the fill character for the char() constructor to another value.  This is then reflected in the output.


octave:2> string_fill_char ('X')
octave:3> double ([char(ones(0,4)); 'A'])
ans =

   65   88   88   88

octave:4> char (ans)
ans = AXXX


Offhand, it would seem that the constructor for char() is firing regardless of whether the dimensions indicate that it should be an empty matrix. 

Rik <rik5>
Group administrator
Tue 28 Nov 2017 06:03:05 AM UTC, comment #3: 

I can sort of see how the parser might get to what you are suggesting.  The examples below are somewhat confirmatory of your hypothesis that the array size is being set before the concatenation takes place.


## In this case, total size is 2 + 1 rows = 3
## empty char matrix is filled out with blanks so 3x1 array of spaces
double ([char(ones(2,0)); 'A'])
ans =

   65
   32
   32




## In this case, the row vector orientation takes precedence and a 1x4
## array is created, and then the first entry is replaced with 'A'
double ([char(ones(0,4)); 'A'])
ans =

   65   32   32   32



Rik <rik5>
Group administrator
Tue 28 Nov 2017 05:27:35 AM UTC, comment #2: 

With the space after char, it's the same as if you wrote


[char, ones(1,0); 'A']


Without looking at the actual code that builds the array, it seems to me that it is incorrectly calculating the size of the result array (filled with blanks, ASCII 32), then filling in elements.  It appears to be counting the rows of the first empty array when computing the result size, but then when it inserts the empty array, there is nothing to insert, so the 'A' ends up in the first position and the second element is left with its initial value of SPC.

John W. Eaton <jwe>
Group administrator
Tue 28 Nov 2017 01:07:17 AM UTC, comment #1: 

It's even weirder than it first appears.  Introduce a space after 'char' and it creates the correct output, although there is a warning.


octave:1> double([char(ones(1,0)); 'A'])
ans =

   65
   32

octave:2> double([char (ones(1,0)); 'A'])
warning: implicit conversion from numeric to char
ans =  65


Marking as confirmed.  Seems like there is something very odd, possibly in the parser.


Rik <rik5>
Group administrator
Mon 27 Nov 2017 11:57:39 PM UTC, original submission:  

These three lines should give the same:


double([ones(1,0); 'A'])
double([char(ones(1,0)); 'A'])
double(vertcat(char(ones(1,0)), 'A'))


However, the results are respectively


ans =  65
ans =

   65
   32

ans =  65


(or see it online in this link: https://tio.run/##y08uSSxL/f8/Jb80KSdVIzo/L7VYw1DHQNNaQd1RPVaTCyaRnJFYpAGXRZcuSy0qSU4s0UBTpQNSpan5/z8A)

How does that space (character 32) crawl in at the end of the second result?

The output in Matlab R2017a is just `65` in all three cases.

Luis Mendo <lmendo>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #42512:  bug52542.cset added by rik5 (2KiB - 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 jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lmendo (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-30 rik5 StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-11-28 rik5 Attached File- Added bug52542.cset, #42512
        StatusConfirmed Patch Submitted
    2017-11-28 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code