bugGNU Octave - Bugs: bug #63965, perms.m doesn't do...

 
 

bug #63965: perms.m doesn't do "unique" for cell inputs

Submitter:  Arun Giridhar <arungiridhar>
Submitted:  Mon 27 Mar 2023 09:00:23 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Wont Fix Assigned to:  arungiridhar
Originator Name:  Open/Closed:  * Closed
Release:  * stable Operating System:  * Any
Fixed Release:  9.1.0 Planned Release:  9.1.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 29 Mar 2023 12:13:01 AM UTC, comment #5: 

Closed as Won't Fix (for stable), since it has been fixed on default.

If anyone needs this functionality for Octave 8, create a numeric index vector corresponding to your cells, and you can permute that with the "unique" option.

Arun Giridhar <arungiridhar>
Group Member
Tue 28 Mar 2023 04:22:44 PM UTC, comment #4: 

This changeset https://hg.savannah.gnu.org/hgweb/octave/rev/7782d1ead0a0 from bug #63962 fixes this bug on default, which will show up in Octave 9.

Since the m-file perms.m will be deleted when transiting from Octave 8 to Octave 9, I am inclined to close this bug as "Fixed on default" / "Won't fix on stable", but if anyone feels that it needs to be fixed on stable, pls comment here.

Arun Giridhar <arungiridhar>
Group Member
Mon 27 Mar 2023 07:19:16 PM UTC, comment #3: 


octave:1> perms ({"foo", "b", "b"}, "unique")
warning: unique: "rows" is ignored for cell arrays
warning: called from
    unique at line 135 column 7
    perms at line 114 column 9

ans =
{
  [1,1] = b
  [2,1] = foo
}



octave:2> unique (perms ({"foo", "b", "b"}), "rows")
warning: unique: "rows" is ignored for cell arrays
warning: called from
    unique at line 135 column 7

ans =
{
  [1,1] = b
  [2,1] = foo
}


The patch attached in comment #1 works for small inputs by exhaustive checks but a better solution is needed, one that can accommodate large inputs as well.

Arun Giridhar <arungiridhar>
Group Member
Mon 27 Mar 2023 06:46:14 PM UTC, comment #2: 

AFAIK 'unique' can process cellstr arrays right away, but no mixed-type or numeric contents. For numeric cell arrays use cell2mat first.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 27 Mar 2023 02:06:02 PM UTC, comment #1: 

Here's a WIP for review (attached).

This is very ugly. Is there a better way?

The core problem is that unique (C) gives an error when C is a cell array. What is the correct way to do a unique on a cell array?


(file #54533)

Arun Giridhar <arungiridhar>
Group Member
Mon 27 Mar 2023 09:00:23 AM UTC, original submission:  

While reviewing bug #63962 I realized that perms.m does not do unique correctly for cell arrays. That is,  perms (C, "unique") isn't correct when C is a cell array. There are also warnings about "rows" and other internal implementation details.

The workaround is to create a numeric index array for C, replacing the element with the index, with identical entries having the same index. Then pass that index to perms and use the result to index back into C. But perms should do that by itself.

I'll work on it this week.

Arun Giridhar <arungiridhar>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54533:  wip.patch added by arungiridhar (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by arungiridhar (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
    2023-03-29 arungiridhar Planned Release8.2.0 9.1.0
    2023-03-29 arungiridhar Fixed ReleaseNone 9.1.0
    2023-03-29 arungiridhar StatusNone Wont Fix
        Open/ClosedOpen Closed
    2023-03-27 arungiridhar Attached File- Added wip.patch, #54533

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code