bugGNU Octave - Bugs: bug #41796, Generalize sub2ind and ind2sub or...

 
 

bug #41796: Generalize sub2ind and ind2sub or provide specialized functions

Submitter:  Juan Pablo Carbajal <juanpi>
Submitted:  Thu 06 Mar 2014 11:00:03 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  In Progress Assigned to:  None
Originator Name:  juanpi Open/Closed:  * Open
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 02 Jul 2016 12:17:31 AM UTC, comment #8: 

Eduardo's patch only applies to sub2ind, not ind2sub.

The attached patch is freshened to apply to the current head, has a few bugs fixed, and probably has a few new ones instead...  I'm not sure if the string_value() call should be xstring_value().

I'm retagging "patch submitted" -> "in progress", since this isn't ready for 4.2.0.

(file #37657)

Lachlan Andrew <lachlan>
Wed 19 Mar 2014 07:49:17 AM UTC, comment #7: 

@Juan Pablo: yes, I was, but it seems that someone did it before me. :)

Actually, I do not know wether it could be worth integrating the two patches or the one submitted by Eduardo is enough. I have a proclivity for the second choice: the parsing is a little bit different, since as far as I have understood the last patch fixes the order of the parameters, but in my opinion this is not such a serious restriction.

Massimiliano Fasi <mfasi>
Wed 19 Mar 2014 04:42:17 AM UTC, comment #6: 

I have implemented the sub2ind for work with triangular 2D matrices along with some tests and documentation. Some of the details regarding implementation are:

1) Dimensions vector must represent a 2D square matrix
2) Indexes of elements above or below diagonal, for lower and upper matrices respectively, are not allowed.
3)  An offset can be given like in tril and triu.It can be negative or positive.
4) "tril" or "triu" should be given as the 4rth argument. Since  we work in 2D it is mandatory that before the string argument, 3 parameters appear.
+markup+
sub2ind(dim, i, j, ["tril"|"triu], offset) Looks like that...
-markup-

I have tried to touch sub2ind function the less I could. Just one if block has been added along with the implementation of a function called sub2ind_triag. That makes possible if needed to remove easily the code in a future.

Here is the patch (file #30945)

Eduardo Ramos Fernández <edu159>
Tue 18 Mar 2014 04:11:10 PM UTC, comment #5: 

Thanks Jordi for updating the report.
The patch submitted just adds the argument checking to sub2ind, but doesn't implement the new functionality (i.e. sub2ind_tril).

@Max: Are you planning to implement the function in C++?

Juan Pablo Carbajal <juanpi>
Group Member
Tue 18 Mar 2014 10:35:24 AM UTC, comment #4: 

I have submitted a new changeset with comments. Of course, you were right, so I switched the two branches of the last if :-).

(file #30939)

Massimiliano Fasi <mfasi>
Mon 17 Mar 2014 08:25:27 PM UTC, comment #3: 

Hi Max,

The changes look goo, just a few comments.
It will be awesome to have some inline comments on the parsing part of the code. This can get big and ugly quite fast, so better keep it commented.

Also, it seems that the if on triangular is inverted, right? One should call the triangular version of ind2sub and sibling when triangular is true.



Juan Pablo Carbajal <juanpi>
Group Member
Sat 08 Mar 2014 04:44:15 PM UTC, comment #2: 

oops, I had a non-so-smart copy/paste, sorry. Can someone change a previous post or it is too late?

[:%s/markup/verbatim]

Massimiliano Fasi <mfasi>
Sat 08 Mar 2014 04:40:58 PM UTC, comment #1: 

I have started working on the problem, modifying the function sub2ind in
+markup+
octave/libinterp/corefcn/sub2ind.cc
-markup-
so that it is able to parse the arguments as required [see submitted patch]. If somebody can check my idea and approve it, I can extend the same approach to the function ind2sub in the same file. After this, I could implement the functions in
+markup+
octave-mechanics/inst/core/private/ind2sub_tril.m
octave-mechanics/inst/core/private/sub2ind_tril.m
-markup-
and put them in
+markup+
octave/liboctave/array/Array-util.cc
-markup-
that is the file that already contains the C++ implementation of sub2ind and ind2sub. Any feedback will be appreciated :-)

(file #30828)

Massimiliano Fasi <mfasi>
Thu 06 Mar 2014 11:00:03 PM UTC, original submission:  

extend sub2ind and ind2sub

It could be useful to extend sub2ind and ind2sub to work with triangular matrices. Currently private functions of the mechanics package.
A string argument triggers the behavior. The string argument can be "triu" or "tril". After this str argument we accept an extra scalar value indicating that the triangular matrix is off diagonal (as in tril and triu).

So, the first modification to the C++ code will be
# Check if any of the argument is a str.
# Check what kind of triangular matrix (up or low).
# If the argument after the str argument is a scalar, it indicates off diagonal triangular matrix (we do not have algo for that yet, though).

This changes should keep the compatibility with Matlab functions.

Juan Pablo Carbajal <juanpi>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #30945:  sub2ind_patch.diff added by edu159 (8KiB - text/x-diff - That maybe could works...)
file #30939:  sub2ind_interface2.patch added by mfasi (3KiB - text/x-diff)
file #30828:  sub2ind_interface.patch added by mfasi (3KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by edu159 (Updated the item)
  • -email is unavailable- added by jordigh (Updated the item)
  • -email is unavailable- added by mfasi (Updated the item)
  • -email is unavailable- added by juanpi (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-02 lachlan Attached File- Added bug_41796_sub2ind_tri.cset, #37657
        StatusPatch Submitted In Progress
    2014-03-19 edu159 Attached File- Added sub2ind_patch.diff, #30945
    2014-03-18 jordigh StatusNone Patch Submitted
    2014-03-18 mfasi Attached File- Added sub2ind_interface2.patch, #30939
    2014-03-08 mfasi Attached File- Added sub2ind_interface.patch, #30828

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code