bugGNU Octave - Bugs: bug #59899, colon operator returns...

 
 

bug #59899: colon operator returns incompatible class for integer inputs

Submitter:  Hartmut <hardy>
Submitted:  Sun 17 Jan 2021 07:31:07 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
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

Sat 06 Mar 2021 02:12:39 PM UTC, comment #10: 

I have compiled dev Octave (hg id 65979d33f147) directly after Rik's changeset mentioned in comment #9. My results

  • make check gives me no failing tests
  • the originally intended behavior (comment #0) works fine
  • I observe the same (in my eyes usefully enough) error messages as mentioned in comment #9.


So I would consider this bug to be fully solved. But maybe other people would like to do further tests.

Hartmut <hardy>
Fri 05 Mar 2021 04:49:34 PM UTC, comment #9: 

I fixed the final issue in this bug report (emitting an error when mixed integer and floating point inputs to colon operator are incompatible) in this changeset http://hg.savannah.gnu.org/hgweb/octave/rev/65979d33f147.

Matlab emits different error messages when a non-integer is used versus a situation where the value is outside of the range of the integer type.  I didn't think we needed to be that specific so I just emit one error message for both situations.

For example,


uint8(1):5.5
error: colon operator upper bound invalid (not an integer or out of range for given integer type)
uint8(1):300
error: colon operator upper bound invalid (not an integer or out of range for given integer type)


Marking as Ready for Test.

Rik <rik5>
Group administrator
Sun 14 Feb 2021 02:26:07 AM UTC, comment #8: 

I addressed comment #3 in this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/c75996cbe3ba).  Octave now distinguishes invalid data types (like a cell array {1}:5) and incompatible data types (int8(1):int32(5)).

I also added xtest BIST tests for the remaining two issues in this report so that it is not forgotten.

Rik <rik5>
Group administrator
Tue 19 Jan 2021 05:18:40 PM UTC, comment #7: 

@jwe: Yes, as far as I understand it that is correct.

This out of range error is not possible because the first error (incompatible types) would be detected first.


rng = uint8 (5) : -1 : int8 (-2)



Rik <rik5>
Group administrator
Tue 19 Jan 2021 05:08:34 PM UTC, comment #6: 

Those additional checks only need to happen if mixing integers with doubles because the integers can't be out of range for their types, correct?

John W. Eaton <jwe>
Group administrator
Tue 19 Jan 2021 05:00:54 PM UTC, comment #5: 

So, Octave is actually closer than I thought.  It seems that mostly the input validation needs to be beefed up.

Case #1 : non-integer inputs in combination with integers


rng = uint8 (0) : 0.4 : 5
rng = uint8 (0) : 5.5


Both of these should produce an error as either the increment or the end point is not an integer.

Case #2 : out-of-range values in combination with integers


rng = -2 : uint8 (5)
rng = uint8 (5) : -1 : -2


Both of these should produce an error as one of the endpoints is out-of-range for the specified datatype.


Rik <rik5>
Group administrator
Mon 18 Jan 2021 06:32:40 PM UTC, comment #4: 

In Matlab R2020b:

>> uint8(10):-1:uint8(1)

ans =

  1×10 uint8 row vector

   10    9    8    7    6    5    4    3    2    1

>> x = uint8(1) : 0.4 : uint8(5)
Error using  :
Double operands interacting with uint8 operands must have integer values.

>> x = uint8 (1 : 0.4 : 5)

x =

  1×11 uint8 row vector

   1   1   2   2   3   3   3   4   4   5   5

>> x = uint8(1):int32(5);
Error using  :
Colon operands must be all the same type, or mixed with real scalar doubles.

>> x = uint8(5) : -1 : -5
Error using  :
Colon operands must be in the range of the data type.

>> x = single(0):pi:100*pi;
>> sprintf ('%.17f\n', x(2))

ans =

    '3.14159274101257324
     '

>> sprintf ('%.17f\n', pi('single'))
Error using pi
Too many input arguments.

>> sprintf ('%.17f\n', single(pi))

ans =

    '3.14159274101257324
     '

>> sprintf ('%.17f\n', double(pi))

ans =

    '3.14159265358979312
     '


Markus Mützel <mmuetzel>
Group administrator
Mon 18 Jan 2021 05:57:12 PM UTC, comment #3: 

@jwe: oops, forgot to check this on dev.  You're right that the original example works.  Would still be useful to understand how the other examples I mentioned work on Matlab.

In Octave I see that


x = uint8(1):int32(5);
error: invalid types found in range expression


It might be nicer to indicate "incompatible types" rather than "invalid types" since that is the true problem.

Rik <rik5>
Group administrator
Mon 18 Jan 2021 04:26:53 PM UTC, comment #2: 

In the current development version sources (what will become Octave 7) we already support integer ranges.  The changes were made in a series of changesets around 5 months ago refactoring the way ranges are handled, beginning with af361aea02e0.

The rules for deciding the result type are in the get_colon_op_type function in ov.cc.

My understanding was that unless the colon operator is used with a class or classdef object, then all arguments must be the same type or mixed with double values.  So if you have a range that is


uint8(10):-1:uint8(1)


the result type would be uint8.  Is that not what Matlab does?



John W. Eaton <jwe>
Group administrator
Mon 18 Jan 2021 03:56:53 PM UTC, comment #1: 

It's true that octave considers all ranges to be ranges of doubles.  Changing this to be Matlab-compatible will be disruptive so any changes will need to take place on the development branch.

It would be helpful to understand whether Matlab internally represents the range as doubles and only converts to the specified class at the end of the calculation, or whether it does all calculations is the specified class.

Could you try this in Matlab?


x = uint8(1) : 0.4 : uint8(5)


If it does everything in the same class than uint8 (0.4) will be zero and this will be the empty matrix.  Otherwise, this will be the same as


x = uint8 (1 : 0.4 : 5)


Which is a sequence with duplicate values, but otherwise valid.

What happens when the classes differ such as


x = uint8(1):int32(5);
class (x)


Does Matlab choose the beginning class or the ending class?

What happens if the endpoints of the range exceed the capacity of the class?


x = uint8(5) : -1 : -5


Seems like there are a lot of corner cases here.  On the other hand, ranges for integers are very easy to implement.  Octave has a sophisticated algorithm to try and get floating point numbers to behave as people think they should regardless of round-off errors, etc.  It might be possible to make the Range class in liboctave a templated class with specializations for floating point.  Would it need a specialization for 'single' types as well or can the algorithm for 'double' be used with a cast to single at the end?

Could you try this


x = single(0):pi:100*pi;
sprintf ('%.17f\n', x(2))
sprintf ('%.17f\n', pi('single'))
sprintf ('%.17f\n', pi('double'))


This should see what increment Matlab used.

Rik <rik5>
Group administrator
Sun 17 Jan 2021 07:31:07 PM UTC, original submission:  

Here is a small demo code to show what I mean:


vec = uint8(1) : uint8(5)
class(vec)


In Matlab the result is 'uint8', but in current Octave I get the result 'double'. (The behavior for uint16 and int16 is the same as for uint8 in Octave.)

The current Octave result is very unexpected to me (in fact it took me several years to figure this out in bug #55059) and it is incompatible to Matlab.

Hartmut <hardy>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by hardy (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
    2021-03-06 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-03-05 rik5 StatusConfirmed Ready For Test
    2021-01-18 rik5 StatusNone Confirmed
        Release6.1.0 dev

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code