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 MarkupRich 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-cleanup