Re: unique semantics

[Rik]

On 03/30/2016 12:40 AM, address@hidden wrote:

Subject:

Making unique() semantics compatible with MATLAB

From:

Danny Goodman <address@hidden>

Date:

03/29/2016 09:51 PM

To:

address@hidden

List-Post:

<mailto:address@hidden>

Precedence:

list

MIME-Version:

1.0

Message-ID:

<address@hidden>

Content-Type:

multipart/alternative; boundary=001a11c1bcbc64aaa1052f3ce758

Message:

5

Hi Octave Developers,

A MATLAB function I tried to run just broke because the 'unique()' function behaves differently between Octave and MATLAB. Specifically, consider this example:

A = [9 2 9 5];
[C1, ia1, ic1] = unique(A)

MATLAB returns column vectors for ia1, ic1 but Octave returns row vectors. Being new to this mailing list, I wanted to ask for advice before submitting a patch to fix this. Am I right that one of the goals of the Octave project is to be able to run MATLAB code as is?

Thanks,
Danny

Danny,

There are two different issues here as pointed out by Ben.  Accordingly, can you file two different bug reports on the bug tracker (https://savannah.gnu.org/bugs/?group=octave)?  The first issue is that Matlab has apparently decided to return column vectors for ia1, ic1 regardless of the input vector orientation.  I think this is less clear than Octave's existing behavior.  Although Octave/Matlab will happily use a column vector as an index into a row vector, I think humans find it easier to match up row to row or column to column.

Using the given example, this reads easier to me because I can index ia1 into A myself by putting it on top of A in my head:
A = [9 2 9 5];
ia1 = [1 2 4];
C = A(ia1) == [9 2 5]

Nevertheless, it's a small gripe and Octave can be made compatible very easily.  Somewhere in unique.m you will need code like this

--- Code ---
if (isvector (i))
  i = i(:);  # always return column vector for Matlab compatibility
endif
--- End Code ---

There will be a similar stanza for the j return value.

The second issue is that Matlab now defaults to returning the first of any duplicate indices, rather than the last.  That is also a trivial fix by switching the internal default from "last" to "first".

You can upload a patch to the bug report to have it reviewed.  Please also update the NEWS file at the top level announcing the change in semantics.  Also, run "test unique" before submitting to make sure that the changes haven't broken anything.  In this case, because you are changing the default behavior some of the BIST tests will certainly break and need to be updated.

Thanks,
Rik




  In the example,