bugGNU Octave - Bugs: bug #45171, isequal fails for large sparse...

 
 

bug #45171: isequal fails for large sparse matrices (subscript indixes too large)

Submitted by:  Marco Caliari <caliari>
Submitted on:  Sat 23 May 2015 11:26:25 AM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  Marco Caliari <caliari>
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  GNU/Linux

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)

Fri 23 Mar 2018 09:45:33 PM UTC, comment #10:

@Mike: If you notice something else, let me know.

Kai Torben Ohlhus <siko1056>
Project Member
Fri 23 Mar 2018 09:25:33 PM UTC, comment #9:

So sorry for the noise, I am testing too many things at once. I must have been accidentally running the new tests but with the old isequal.m function in the load path. The tests do pass. Closing again.

Mike Miller <mtmiller>
Project Administrator
Fri 23 Mar 2018 09:13:48 PM UTC, comment #8:

Kai - this change (0e5ae3ecda1a) on stable introduces 6 new tests, all of which fail on Octave built for 32-bit i386 with an "out of memory or dimension too large" error. Can you take a look at that?

Mike Miller <mtmiller>
Project Administrator
Thu 22 Mar 2018 03:17:45 PM UTC, comment #7:

This is still a problem in the current stable. I updated the original patch file #34084 to meet the current implementation and pushed it: http://hg.savannah.gnu.org/hgweb/octave/rev/0e5ae3ecda1a

Kai Torben Ohlhus <siko1056>
Project Member
Fri 18 Nov 2016 07:15:18 PM UTC, comment #6:

I would venture to say that numel() is preferred over length() in the Octave code base these days even when it is guaranteed that the variable references a vector.

Mike Miller <mtmiller>
Project Administrator
Thu 17 Nov 2016 08:38:43 AM UTC, comment #5:

Hi, now that 4.2 is out I'd like to apply this change.
Can it be pushed as is or should I replace length by numel?

Carlo de Falco <cdf>
Project Member
Wed 28 Sep 2016 02:54:51 PM UTC, comment #4:

x is the result of

so even if the original x was a matrix after this call
it will be a vector.

similarly for y below in the code

nonetheless I see no problem in using numel instead of length if you prefer ...

Carlo de Falco <cdf>
Project Member
Wed 28 Sep 2016 02:49:52 PM UTC, comment #3:

I agree that in the original code, length is OK because it is applied to the result of the find function. But in the modified version it is applied to X. Can that be a matrix?

John W. Eaton <jwe>
Project Administrator
Wed 28 Sep 2016 11:05:32 AM UTC, comment #2:

This patch was being discussed on IRC yesterday,
when my connection went suddenly off, I'll
continue the discussion here.

Just before communication was interrupted jwe asked:

My answer would have been:

I would say length is equivalent to numel in this context because find should always return vectors, as noted in its
documentation.

But if you prefer it I see no problem in replacing length by numel in the patch.

Also note that the use of length was there before this change.

Carlo de Falco <cdf>
Project Member
Thu 22 Sep 2016 09:04:36 AM UTC, comment #1:

This report is quite old, but by browsing for "__isequal__" it got my attention. Your problem does not exist, if you are using 64-bit indices, as you are touching the critical border:

What prevents you from applying your patch before the 4.2 release?

Kai Torben Ohlhus <siko1056>
Project Member
Sat 23 May 2015 11:26:25 AM UTC, original submission:

Dear all,

the following code

fails with

It is the already seen problem of linear indexing of large sparse matrices. On the other hand, I was able to fix this case with the enclosed patch, still preserving the 26 tests in isequal.m.

Cheers,

Marco

Marco Caliari <caliari>
Project MemberIn charge of this item.

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #34084:  __isequal__.m.patch added by caliari (673B - 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 mtmiller (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by cdf (Posted a comment)
  • -email is unavailable- added by cdf
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by caliari (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-03-23 mtmiller StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2018-03-23 mtmiller StatusFixed => In Progress
        Open/ClosedClosed => Open
    2018-03-22 siko1056 StatusPatch Submitted => Fixed
        Open/ClosedOpen => Closed
        Release4.2.0 => dev
    2016-11-17 mtmiller Release4.2.0-rc2 => 4.2.0
    2016-09-28 cdf Carbon-Copy- => Added -email is unavailable-
    2016-09-22 siko1056 Item GroupNone => Incorrect Result
        StatusNone => Patch Submitted
        Assigned toNone => caliari
        Release3.8.2 => 4.2.0-rc2
    2015-05-23 caliari Summaryisequal fails for large sparse matrices (subscript indices too large) => isequal fails for large sparse matrices (subscript indixes too large)
    2015-05-23 caliari Attached File- => Added _isequal_.m.patch, #34084

    Back to the top


    Powered by Savane 3.3