bugGNU Octave - Bugs: bug #52437, ismember fails for complex input

 
 

bug #52437: ismember fails for complex input

Submitter:  Luis Mendo <lmendo>
Submitted:  Fri 17 Nov 2017 10:43:57 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Luis Mendo Open/Closed:  * Closed
Release:  * 4.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 15 Oct 2019 12:24:43 AM UTC, comment #8: 

@Paco: Instead of posting to a bug report that thas been closed for a year and a half it is better to report a new bug.  I saw this only through your request to join the Octave Maintainers group, which isn't necessary.

I checked in a fix on the stable branch here: https://hg.savannah.gnu.org/hgweb/octave/rev/616b844e903a.

You can download the m-file from the Mercurial repository if you need the fix immediately.  Otherwise, it will be a part of the next bug fix release of Octave.

Rik <rik5>
Group administrator
Wed 17 Jul 2019 06:15:07 PM UTC, comment #7: 

This code

[~,b] = ismember(-1-1j, [-1-1j, -1+3j, -1+1j])

yields 3 instead of 1

This is due to ismember using only the real part for computing the index, and lookup returning the last index in case of values present more than once

Paco Leon <fleon11>
Thu 18 Jan 2018 01:24:56 PM UTC, comment #6: 
Carnë Draug <carandraug>
Group Member
Thu 18 Jan 2018 12:50:49 PM UTC, comment #5: 

It's not only the second output that fails, the first fails as well. This bug was already present in version 4.0 and 4.2:



octave-gui:1> [a, b] = ismember ([5 4-3j 3+4j], 5)
a =

  1  1  1

b =

   1   1   1


Carnë Draug <carandraug>
Group Member
Sat 18 Nov 2017 06:24:06 PM UTC, comment #4: 

A possible patch to the reimplemented version is to add something like this at the beginning:


if (isnumeric(a)&&~isreal(a)) || (isnumeric(s)&&~isreal(s))
  [~, ~, u] = unique([a(:); s(:)]);
  a(:) = real(u(1:numel(a)));
  s(:) = real(u(numel(s)+1:end));
end


That is, if complex inputs are detected, replace values in the inputs by globally unique integer labels. By "globally unique" I mean unique across the two inputs. That way the lookup function will work fine.

Luis Mendo <lmendo>
Sat 18 Nov 2017 04:56:02 PM UTC, comment #3: 

Thanks.

Note that the linked version before the reimplementation gives an error on logical inputs. Matlab and Octave's reimplemented version work with logical inputs.

So in addition to regression, something like this could be added at the beginning of ismember:


if islogical(a), a = uint8(a); end
if islogical(s), s = uint8(s); end


Also, Matlab's and Octave's reimplemented ismember are happy to compare inputs of different numeric type, as long as one of them is double in Matlab, or in any case in Octave. Replacing the following line in ismember (version before the reimplementation)


if (! isa (a, class (s)))


by


if (! isa (a, class (s))) && ...
  ! (isa (a, 'double') || isa (s, 'double'))


seems enough.

Luis Mendo <lmendo>
Sat 18 Nov 2017 06:34:56 AM UTC, comment #2: 

Apparently this cset http://hg.octave.org/octave/rev/dbc61d4e428d broke ismember for complex input.

Before the "reimplementation" ismember returned [1 2 3], you can try with this http://hg.octave.org/octave/raw-file/0c4e6a3d6e3e/scripts/set/ismember.m

mark as regression

Andreas Weber <andy1978>
Group Member
Sat 18 Nov 2017 01:09:11 AM UTC, comment #1: 

To clarify: ismember apparently was "reimplemented using lookup  and unique" (as indicated by the first comment I quoted, taken from its source code). And lookup doesn't handle complex values properly (as indicated by the second comment I quoted, taken from its source code).

Luis Mendo <lmendo>
Fri 17 Nov 2017 10:43:57 PM UTC, original submission:  

This code


x = [5 4-3j 3+4j]; [~, b] = ismember(x, x)


gives [3 3 3] instead of the expected [1 2 3].

The problem seems to be that 'ismember' uses the 'lookup' function, which was


## Reimplemented using lookup & unique: Jaroslav Hajek <highegg@gmail.com>


and 'lookup' has the comment


// In the case of a complex array, absolute values will be used for
// compatibility (though it's not too meaningful).


Well, it may not be meaningful for lookup's original purpose, but it definitely is for ismember.

It looks like ismember's second output should be obtained with something other than lookup, at least in the complex case

Luis Mendo <lmendo>

 

(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 rik5 (Posted a comment)
  • -email is unavailable- added by fleon11 (Posted a comment)
  • -email is unavailable- added by andy1978 (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
    2018-01-18 carandraug StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2018-01-18 carandraug SummarySecond output of ismember is wrong for complex input ismember fails for complex input
    2017-11-18 andy1978 Item GroupIncorrect Result Regression
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.14-f13d.
    Corresponding source code