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

 
 

patch #8066: speedup of strjoin

Submitter:  None
Submitted:  Tue 04 Jun 2013 05:51:27 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  bpabbott Originator Email:  -email is unavailable-
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 23 Jun 2013 07:25:14 AM UTC, comment #12: 
Ben Abbott <bpabbott>
Group Member
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: 
Ben Abbott <bpabbott>
Group Member
Sun 23 Jun 2013 12:51:16 AM UTC, comment #9: 

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


str = strjoin ({}, '*')

str =

     ''

whos str
  Name      Size            Bytes  Class    Attributes

  str       0x0                 0  char


Ben Abbott <bpabbott>
Group Member
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>
Group Member
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>
Group Member
Sun 16 Jun 2013 03:04:22 AM UTC, comment #3: 

A problem

From Matlab


strjoin({'Octave','Scilab'}, '\n')

ans =

Octave
Scilab


From Octave


strjoin({'Octave','Scilab'}, '\n')
ans = Octave\nScilab


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


strjoin({'Octave\n','Scilab\n'}, 'foo')

ans =

Octave\nfooScilab\n


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


strjoin({'Octave','Scilab'}, do_string_escapes ('\n'))


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

Ben Abbott <bpabbott>
Group Member
Sat 15 Jun 2013 11:04:49 AM UTC, comment #2: 
Ben Abbott <bpabbott>
Group Member
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 Files:
   
   
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
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by dac922 (Posted a comment)
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-08-03 bpabbott StatusIn Progress Done
    2013-08-03 bpabbott Open/ClosedOpen Closed
    2013-06-16 dac922 Attached File- Added strjoin_escape_sequences.patch, #28327
    2013-06-16 bpabbott StatusReady For Test In Progress
    2013-06-15 bpabbott StatusNone Ready For Test
        Assigned toNone bpabbott
    2013-06-04 None Attached File- Added strjoin.patch, #28264

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code