patchGNU Octave - Patches: patch #8066, speedup of strjoin

 
 

patch #8066: speedup of strjoin

Submitted by:  None
Submitted on:  Tue 04 Jun 2013 05:51:27 PM UTC  
 
Category: NonePriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Ben Abbott <bpabbott>Originator Email: -unavailable-
Open/Closed: Closed

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)

Sun 23 Jun 2013 07:25:14 AM UTC, comment #12:

Thanks. I've added the semicolon.

http://hg.savannah.gnu.org/hgweb/octave/rev/893e9cec62f1

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 23 Jun 2013 05:14:24 AM UTC, comment #11:

OK, thanks. An error didn't seem like the right thing to do.

I just tried the change. Line 17 needs a semicolon at the end to keep the output silent:

http://hg.savannah.gnu.org/hgweb/octave/rev/7a97ff5ef42e#l1.17

Dan Sebald <sebald>
Sun 23 Jun 2013 01:21:48 AM UTC, comment #10:

I pushed a changeset

http://hg.savannah.gnu.org/hgweb/octave/rev/7a97ff5ef42e

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 23 Jun 2013 12:51:16 AM UTC, comment #9:

I tried your example in Matlab. Matlab returns an empty string.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sat 22 Jun 2013 04:45:36 PM UTC, comment #8:

In helping debug this report:

https://savannah.gnu.org/bugs/?39288

I came across the following result that I'm wondering about:

>> strjoin ({}, "*")

error: strjoin: the number of delimiters does not match the number of strings
error: called from:
error: /home/sebald/matlab/strjoin.m at line 74, column 5

Dan Sebald <sebald>
Sun 16 Jun 2013 12:01:14 PM UTC, comment #7:

Stephan,

I confirmed you reading of the docs. Your changeset looks good.

http://hg.savannah.gnu.org/hgweb/octave/rev/70ea511edbc4

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 16 Jun 2013 10:14:47 AM UTC, comment #6:

The matlab docs say that a delimiter with type cell array doesn't support escape sequences, so this simple patch should fix the issue.

(file #28327)

Stefan Mahr <dac922>
Sun 16 Jun 2013 09:50:49 AM UTC, comment #5:

This is a Matlab compatibility issue that appears with the old version of strjoin as well. Should we open a new bugreport instead?

Could you check if strings within a cell are also escaped?

strjoin({'a','b','c'}, {'\n','\n'})

Stefan Mahr <dac922>
Sun 16 Jun 2013 03:06:42 AM UTC, comment #4:

Stefan, I'm changing this to "in Progress". Please let us know of you'll propose a fix. If not either myself or someone else can do it.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sun 16 Jun 2013 03:04:22 AM UTC, comment #3:

A problem

From Matlab

From Octave

If double-quotes are used, the Octave work correctly because the parser handles the conversion of escaped characters.

Thus, single-quoted delimiters need to be escaped by strjoin(). The first input does not, Matlab again

Unless I'm missing something, a patch is needed that runs do_string_escapes() on the second input. For the example above,

However, since the second input may be a cell-string a proper solution is a bit more complicated.

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Sat 15 Jun 2013 11:04:49 AM UTC, comment #2:

I pushed the changeset

http://hg.savannah.gnu.org/hgweb/octave/rev/c6d61dca5acd

Ben Abbott <bpabbott>
Project MemberIn charge of this item.
Tue 04 Jun 2013 05:53:45 PM UTC, comment #1:

With attached patch:

> % long string, less cells
> a={repmat('a',1,1e5)};
> b=repmat(a,1,1e3);


> tic
> c1=strjoin(b);
> toc

Elapsed time is 0.1559851 seconds.

Anonymous
Tue 04 Jun 2013 05:51:27 PM UTC, original submission:

> % short string, many cells
> a = {'a'};
> b=repmat(a,1,1e5);
> tic
> c1=strjoin(b);
> toc

Elapsed time is 0.363541 seconds.

> tic
> c2=[[b(:).' ; repmat({' '},1,numel(b))]{:}](1:end-1);
> toc

Elapsed time is 0.3507609 seconds.

> % long string, less cells
> a={repmat('a',1,1e5)};
> b=repmat(a,1,1e3);
> tic
> c1=strjoin(b);
> toc

Elapsed time is 1.692134 seconds.

> tic
> c2=[[b(:).' ; repmat({' '},1,numel(b))]{:}](1:end-1);
> toc

Elapsed time is 0.2909841537 seconds.

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #28264:  strjoin.patch added by None (601B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by sebald (Posted a comment)
  • -unavailable- added by dac922 (Posted a comment)
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by None (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 logged-in users can vote.

     

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

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 03 Aug 2013 02:38:25 AM UTCbpabbottStatusIn Progress=>Done
    Sat 03 Aug 2013 02:38:13 AM UTCbpabbottOpen/ClosedOpen=>Closed
    Sun 16 Jun 2013 10:14:47 AM UTCdac922Attached File-=>Added strjoin_escape_sequences.patch, #28327
    Sun 16 Jun 2013 03:06:42 AM UTCbpabbottStatusReady For Test=>In Progress
    Sat 15 Jun 2013 11:04:49 AM UTCbpabbottStatusNone=>Ready For Test
      Assigned toNone=>bpabbott
    Tue 04 Jun 2013 05:51:27 PM UTCNoneAttached File-=>Added strjoin.patch, #28264

    Back to the top


    Powered by Savane 3.1-cleanup1