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

 
 

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

Submitted by:  Juan Pablo Carbajal <juanpi>
Submitted on:  Thu 06 Mar 2014 11:00:03 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Feature Request
Status: In ProgressAssigned to: None
Originator Name: juanpiOpen/Closed: Open
Release: devOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project Member
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>
Project 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>
Project 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>
Project Member

 

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

Attach File(s):
   
   
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
  • -unavailable- added by lachlan (Updated the item)
  • -unavailable- added by edu159 (Updated the item)
  • -unavailable- added by jordigh (Updated the item)
  • -unavailable- added by mfasi (Updated the item)
  • -unavailable- added by juanpi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 02 Jul 2016 12:17:31 AM UTClachlanAttached File-=>Added bug_41796_sub2ind_tri.cset, #37657
      StatusPatch Submitted=>In Progress
    Wed 19 Mar 2014 04:39:47 AM UTCedu159Attached File-=>Added sub2ind_patch.diff, #30945
    Tue 18 Mar 2014 02:25:37 PM UTCjordighStatusNone=>Patch Submitted
    Tue 18 Mar 2014 10:35:24 AM UTCmfasiAttached File-=>Added sub2ind_interface2.patch, #30939
    Sat 08 Mar 2014 04:40:58 PM UTCmfasiAttached File-=>Added sub2ind_interface.patch, #30828

    Back to the top


    Powered by Savane 3.1-cleanup1