octave-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-bug-tracker] [bug #65176] unique.m - Enable third output with op


From: Nicholas Jankowski
Subject: [Octave-bug-tracker] [bug #65176] unique.m - Enable third output with option 'stable'
Date: Tue, 20 Feb 2024 18:03:20 -0500 (EST)

Follow-up Comment #7, bug#65176 (group octave):

no problem with the timing. you've seen how long some bugs sit :)

i did a look through the patch. it seems to work fairly well. A few issues -
octave tends to perform poorly with for loops.  It's not that noticeable with
small inputs, but for larger arrays/loops it can get significant. 

I attached a revised patch where I was able to replace your loops with some
vectorized equivalents.  Still passes all the tests.  
see a quite timing comparison on the two:


## small input borrowed from a BIST, not too much different
A = [4,5,6; 1,2,3; 4,5,6];

#loops
tic; for idx = 1:10000, [y,i,j] = unique(A,"rows","stable");endfor,toc
Elapsed time is 1.78322 seconds.

#vectorized
tic; for idx = 1:10000, [y,i,j] = unique(A,"rows","stable");endfor,toc
Elapsed time is 1.59985 seconds.

## bigger array 
A = randi([1 10], 1000, 3);

#loops
> tic; for idx = 1:100, [y,i,j] = unique(a,"rows","stable");endfor,toc
Elapsed time is 1.523 seconds.

#vectorized
tic; for idx = 1:100, [y,i,j] = unique(a,"rows","stable");endfor,toc
Elapsed time is 0.121731 seconds.


I also cleaned up some of the formatting (mostly whitespace around functions
and operators - see the style guide link below.), separated two added tests
and added a <bug#> tag to them so the test suite can trace them back if a
regression ever occurs.


I'm still wondering if that could get streamlined a bit more.  There are a few
calls to sort that I think might be redundant with the multi-level mapping,
and my find(u(s)==j') to project into dim2 followed by cleanup math is a bit
of a hack that might not be necessary with the right mapping.   I left the old
code in as comments for now as a reminder, but they should get trimmed before
being finished. let me know what you think.

also, assuming we push this for octave 10 in the near future, would you like
to be in the contributor list (assuming your name as shown here and with email
from your savannah profile?)

(file #55727)

    _______________________________________________________

Additional Item Attachment:

File name: unique_stable_bug65176_v3.patch Size:7 KB
   
<https://file.savannah.gnu.org/file/unique_stable_bug65176_v3.patch?file_id=55727>


    AGPL NOTICE

These attachments are served by Savane. You can download the corresponding
source code of Savane at
https://git.savannah.nongnu.org/cgit/administration/savane.git/snapshot/savane-24d3702d7a77e2ff4eb0f39b986d772d82c134fd.tar.gz


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?65176>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]