www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Too complicated code for generating a random string?

reply Jens Mueller <jens.k.mueller gmx.de> writes:
Hi,

I'd like to sample with replacement. But found no simple way.
In particular I want to generate a random string of given letters, say
std.ascii.letters.

Anybody a simpler version than

auto randomString = repeat('a').take(10).map!(c => randomSample(letters, 1,
letters.length))().joiner();

?

Jens
Feb 22 2013
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/22/2013 04:23 PM, Jens Mueller wrote:

 Hi,

 I'd like to sample with replacement. But found no simple way.
 In particular I want to generate a random string of given letters, say
 std.ascii.letters.

 Anybody a simpler version than

 auto randomString = repeat('a').take(10).map!(c =>  randomSample(letters, 1,
letters.length))().joiner();

 ?

 Jens


Here is another way:

import std.stdio;
import std.algorithm;
import std.ascii;
import std.random;
import std.range;

auto pickOne(R)(R range)
     // insert template constraints here ... :)
{
     return range[uniform(0, range.length)];
}

void main()
{
     writeln(iota(10).map!((_) => pickOne(letters)));
}

Ali
Feb 22 2013
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Ali Çehreli:


 auto pickOne(R)(R range)
     // insert template constraints here ... :)
 {
     return range[uniform(0, range.length)];
 }


That's the function choice() I'd like in Phobos:
http://d.puremagic.com/issues/show_bug.cgi?id=4851

Note that:
range[uniform(0, range.length)];

is written more compactly as:
range[uniform(0, $)];




 void main()
 {
     writeln(iota(10).map!((_) => pickOne(letters)));
 }


Instead of "(_)" I think it's better to use a simpler "_".

That's another commonly useful function, often named table(), 
similar to map() but doesn't pass an index to the callable:

http://reference.wolfram.com/mathematica/ref/Table.html

So it becomes something like:

10.table!({ return letters.choice; }).writeln;

Bye,
bearophile
Feb 22 2013
next sibling parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/22/2013 05:07 PM, bearophile wrote:

 Ali Çehreli:


 auto pickOne(R)(R range)
 // insert template constraints here ... :)
 {
 return range[uniform(0, range.length)];
 }


 That's the function choice() I'd like in Phobos:
 http://d.puremagic.com/issues/show_bug.cgi?id=4851


Agreed.


 Note that:
 range[uniform(0, range.length)];

 is written more compactly as:
 range[uniform(0, $)];


Good point. I would think that $ must be "out in the open" to work that way.


 void main()
 {
 writeln(iota(10).map!((_) => pickOne(letters)));
 }


 Instead of "(_)" I think it's better to use a simpler "_".


Mine evolved from the earlier buggy ()=> then I inserted an underscore 
without realizing to remove the parentheses.


 That's another commonly useful function, often named table(), similar to
 map() but doesn't pass an index to the callable:

 http://reference.wolfram.com/mathematica/ref/Table.html


I did need such a function in the past. I have been thinking about the 
name 'generate' but if 'table' is common already, why not. :)


 So it becomes something like:

 10.table!({ return letters.choice; }).writeln;

 Bye,
 bearophile


Ali
Feb 22 2013
prev sibling next sibling parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
On 2/22/13 10:07 PM, bearophile wrote:


 That's another commonly useful function, often named table(), similar to
 map() but doesn't pass an index to the callable:

 http://reference.wolfram.com/mathematica/ref/Table.html

 So it becomes something like:

 10.table!({ return letters.choice; }).writeln;


Couldn't map! be used with a delegate that doesn't receive any arguments?
Feb 22 2013
parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Ary Borenszweig:


 10.table!({ return letters.choice; }).writeln;


 Couldn't map! be used with a delegate that doesn't receive any 
 arguments?


I think map() requires a callable that receives one argument.

table() doesn't need the iota() as map(), it accepts one or more 
numerical arguments, so it's able to generate 2D arrays, 3D 
arrays, etc.:

table!(() => letters.choice)(3, 4, 5).writeln;

Bye,
bearophile
Feb 23 2013
parent reply Jens Mueller <jens.k.mueller gmx.de> writes:
bearophile wrote:

 Ary Borenszweig:
 

10.table!({ return letters.choice; }).writeln;


Couldn't map! be used with a delegate that doesn't receive any
arguments?


 
 I think map() requires a callable that receives one argument.
 
 table() doesn't need the iota() as map(), it accepts one or more
 numerical arguments, so it's able to generate 2D arrays, 3D arrays,
 etc.:
 
 table!(() => letters.choice)(3, 4, 5).writeln;


This looks almost like a Cartesian product but only specifying the
sizes.
I assume you have an enhancement request? Couldn't find it.

Jens
Feb 23 2013
parent "bearophile" <bearophileHUGS lycos.com> writes:
Jens Mueller:


 table!(() => letters.choice)(3, 4, 5).writeln;


 This looks almost like a Cartesian product but only specifying 
 the sizes.


table() also takes a function, so it's closer to a map() or to 
minimallyInitializedArray() than to a Cartesian product :-)



 I assume you have an enhancement request? Couldn't find it.


I think I don't have an enhancement request on table().

Bye,
bearophile
Feb 23 2013
prev sibling parent reply greenbyte <lysenko765 yandex.ru> writes:
On Saturday, 23 February 2013 at 01:07:23 UTC, bearophile wrote:

 So it becomes something like:

 10.table!({ return letters.choice; }).writeln;


Dear bearophile, without the table() function in the standard 
library, this code does not work.

After hours, I managed to write code to generate random strings 
based on your code.

```d
import std.array : array;
import std.ascii : letters, digits;
import std.random : choice, Random, unpredictableSeed;
import std.range : generate, take;
import std.stdio : writeln;

auto rnd = Random(unpredictableSeed);
auto symbols = array(letters ~ digits);

generate!({ return symbols.choice(rnd); }).take(10).writeln;

```
Jun 17 2022
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 6/17/22 10:16, greenbyte wrote:

 On Saturday, 23 February 2013 at 01:07:23 UTC, bearophile wrote:


Good old bearophile! :) He has been lured by other programming languages 
(Rust?) long time ago.

Ali
Jun 17 2022
next sibling parent Jesse Phillips <Jesse.K.Phillips+D gmail.com> writes:
On Friday, 17 June 2022 at 20:17:31 UTC, Ali Çehreli wrote:

 On 6/17/22 10:16, greenbyte wrote:

 On Saturday, 23 February 2013 at 01:07:23 UTC, bearophile


 wrote:

 Good old bearophile! :) He has been lured by other programming 
 languages (Rust?) long time ago.

 Ali


that was my same reaction, hey bearophile back and then realized 
an old thread is resurrected.

import std;
void main()
{
    auto rnd = MinstdRand0(42);u, '''
    writeln(letters.array.randomCover(rnd).front);
}
Jun 17 2022
prev sibling parent Abdulhaq <alynch4047 gmail.com> writes:
On Friday, 17 June 2022 at 20:17:31 UTC, Ali Çehreli wrote:

 On 6/17/22 10:16, greenbyte wrote:

 On Saturday, 23 February 2013 at 01:07:23 UTC, bearophile


 wrote:

 Good old bearophile! :) He has been lured by other programming 
 languages (Rust?) long time ago.

 Ali


he has ceased to D

<groan/>
Jun 18 2022
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Ali =C3=87ehreli wrote:

 On 02/22/2013 04:23 PM, Jens Mueller wrote:

Hi,

I'd like to sample with replacement. But found no simple way.
In particular I want to generate a random string of given letters, say
std.ascii.letters.

Anybody a simpler version than

auto randomString =3D repeat('a').take(10).map!(c =3D>  randomSample(let=




ters, 1, letters.length))().joiner();

?

Jens


=20
 Here is another way:
=20
 import std.stdio;
 import std.algorithm;
 import std.ascii;
 import std.random;
 import std.range;
=20
 auto pickOne(R)(R range)
     // insert template constraints here ... :)
 {
     return range[uniform(0, range.length)];
 }
=20
 void main()
 {
     writeln(iota(10).map!((_) =3D> pickOne(letters)));
 }


Thanks that looks better.
Phobos misses a pickOne function.

Jens
Feb 23 2013
prev sibling parent reply Jens Mueller <jens.k.mueller gmx.de> writes:
Ali =C3=87ehreli wrote:

 On 02/22/2013 04:23 PM, Jens Mueller wrote:

Hi,

I'd like to sample with replacement. But found no simple way.
In particular I want to generate a random string of given letters, say
std.ascii.letters.

Anybody a simpler version than

auto randomString =3D repeat('a').take(10).map!(c =3D>  randomSample(let=




ters, 1, letters.length))().joiner();

?

Jens


=20
 Here is another way:
=20
 import std.stdio;
 import std.algorithm;
 import std.ascii;
 import std.random;
 import std.range;
=20
 auto pickOne(R)(R range)
     // insert template constraints here ... :)
 {
     return range[uniform(0, range.length)];
 }
=20
 void main()
 {
     writeln(iota(10).map!((_) =3D> pickOne(letters)));
 }


I think this version also looks good:
void main()
{
	auto randomLetter =3D () =3D> randomSample(letters, 1, letters.length).fro=
nt;
	writeln(iota(10).map!(_ =3D> randomLetter()));
}

Which makes me think Phobos is convenient enough in this use case. Just
finding a simple way to use it can be a burden. It should be improved
with better documentation.

Jens
Feb 23 2013
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Jens Mueller:


 Which makes me think Phobos is convenient enough in this use 
 case.


I don't agree. This is not quite worse
auto randomLetter = () => randomSample(letters, 1, 
letters.length).front;

than:
auto randomLetter = () => letters.choice;

Bye,
bearophile
Feb 23 2013
parent reply Jens Mueller <jens.k.mueller gmx.de> writes:
bearophile wrote:

 Jens Mueller:
 

Which makes me think Phobos is convenient enough in this use case.


 
 I don't agree. This is not quite worse
 auto randomLetter = () => randomSample(letters, 1,
 letters.length).front;
 
 than:
 auto randomLetter = () => letters.choice;


Then add choice explicitly.
auto choice(R)(R r) { return randomSample(r, 1, r.length).front; };

The point is that choice is a one-liner when using randomSample. It's a
special case of randomSample. But you may argue that the use case occurs
that often that it should be added. Don't know whether this is the case.

Jens
Feb 23 2013
parent "bearophile" <bearophileHUGS lycos.com> writes:
Jens Mueller:


 It's a special case of randomSample.


Maybe in theory, but I am not going to use randomSample to 
generate a random string. Your code is slow and bug-prone.



 But you may argue that the use case occurs
 that often that it should be added.


Right. (I even argue that in Phobos today there are functions 
that are much less commonly useful than a choice()).

Bye,
bearophile
Feb 23 2013
prev sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 13:02:38 UTC, Jens Mueller wrote:

 I think this version also looks good:
 void main()
 {
 	auto randomLetter = () => randomSample(letters, 1, 
 letters.length).front;
 	writeln(iota(10).map!(_ => randomLetter()));
 }

 Which makes me think Phobos is convenient enough in this use 
 case. Just
 finding a simple way to use it can be a burden. It should be 
 improved
 with better documentation.

 Jens


No offense, but I think that's horrible. No matter how you put 
it, you should keep in mind that the strength of these ranges is 
being able do stuff lazily.

The final product of what you have is a horrible contraption of 
chained ranges, that go out of their way just to pick a random 
char for you.

Not to mention, you are creating a randomSample per character, 
which is simply not the way it is designed to be used. Generating 
an entire new range just to generate a random letter...? At 
least: use uniform instead:

auto randomLetter = () => letters[uniform (0, letters.length);

Still, even with that, IMO, the end product is bug prone, not 
very clear, and horribly inefficient. I'd hate to be the one to 
have to maintain this down the pipe... And at the end of the day, 
you'd still need to add an "array" at the end if you want to put 
it in a string anyways.

--------
Honestly, what happened to doing it by hand? It's more robust, 
cleaner, easy to understand...


     string randomString;
     {
         auto app = Appender!string();
         foreach(_ ; 0 .. 10)
             app.put(letters[uniform(0, letters.length)]);
         randomString = app.data;
     }

This is the "easy and safe" version. You could make it even more 
simple with a pre-allocate dchar:


     string randomString;
     {
         auto tmp = new dchar[](10);
         foreach(ref c ; tmp)
             c = letters[uniform(0, letters.length)];
         randomString = tmp.to!string();
     }

Or, if you know your letters are all ascii, it gets even more 
simple and efficient:

     string randomString;
     {
         auto tmp = new char[](10);
         foreach(ref char c ; tmp)
             c = letters[uniform(0, letters.length)];
         randomString = cast(string)letters;
     }

All those scope blocks I put in are not mandatory. At the end of 
the day, it is 4 lines of code. Very efficient, and the intent is 
crystal clear.

I know it's not very exciting code, they do say good code is 
boring code.
Feb 23 2013
next sibling parent reply Jens Mueller <jens.k.mueller gmx.de> writes:
monarch_dodra wrote:

 On Saturday, 23 February 2013 at 13:02:38 UTC, Jens Mueller wrote:

I think this version also looks good:
void main()
{
	auto randomLetter = () => randomSample(letters, 1,
letters.length).front;
	writeln(iota(10).map!(_ => randomLetter()));
}

Which makes me think Phobos is convenient enough in this use case.
Just
finding a simple way to use it can be a burden. It should be
improved
with better documentation.

Jens


 
 No offense, but I think that's horrible. No matter how you put it,
 you should keep in mind that the strength of these ranges is being
 able do stuff lazily.
 
 The final product of what you have is a horrible contraption of
 chained ranges, that go out of their way just to pick a random char
 for you.
 
 Not to mention, you are creating a randomSample per character, which
 is simply not the way it is designed to be used. Generating an
 entire new range just to generate a random letter...? At least: use
 uniform instead:
 
 auto randomLetter = () => letters[uniform (0, letters.length);
 
 Still, even with that, IMO, the end product is bug prone, not very
 clear, and horribly inefficient. I'd hate to be the one to have to
 maintain this down the pipe... And at the end of the day, you'd
 still need to add an "array" at the end if you want to put it in a
 string anyways.
 
 --------
 Honestly, what happened to doing it by hand? It's more robust,
 cleaner, easy to understand...
 
 
     string randomString;
     {
         auto app = Appender!string();
         foreach(_ ; 0 .. 10)
             app.put(letters[uniform(0, letters.length)]);
         randomString = app.data;
     }
 
 This is the "easy and safe" version. You could make it even more
 simple with a pre-allocate dchar:
 
 
     string randomString;
     {
         auto tmp = new dchar[](10);
         foreach(ref c ; tmp)
             c = letters[uniform(0, letters.length)];
         randomString = tmp.to!string();
     }
 
 Or, if you know your letters are all ascii, it gets even more simple
 and efficient:
 
     string randomString;
     {
         auto tmp = new char[](10);
         foreach(ref char c ; tmp)
             c = letters[uniform(0, letters.length)];
         randomString = cast(string)letters;
     }
 
 All those scope blocks I put in are not mandatory. At the end of the
 day, it is 4 lines of code. Very efficient, and the intent is
 crystal clear.
 
 I know it's not very exciting code, they do say good code is boring
 code.


I see your point. But I believe it depends on what you want to achieve
and how you are constrained. I do not care about very good efficiency in
this case. I just need a solution that works. I can replace that code
later when needed. Your solution is less precise but much more
efficient.
Can we have a generic function/code that is as efficient as yours? That
is what we are aiming for, right?

My hope is that machine code generated from

auto randomLetter = () => letters[uniform (0, letters.length)];
auto randomString = iota(10).map!(_ => randomLetter()).array();

is on par with hand-written code most of the time. Or at least it's fast
enough for many use cases.

Jens
Feb 23 2013
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 14:28:28 UTC, Jens Mueller wrote:

 Can we have a generic function/code that is as efficient as 
 yours?


Well, for now, we have "map" (and "iota"). This usually covers 
enough ground, but there remains cases where it is sub-optimal.

What we would need is a "generate". Basically, a range that calls 
a function to generate the value of front on the fly. I've seen 
enough threads like this one that "abuse" iota/repeat/take/map 
(or sequence and recurrence for that matter), just to obtain a 
generic generator.

Let's just write Generator/generator.
Feb 23 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/23/13 5:56 PM, monarch_dodra wrote:

 On Saturday, 23 February 2013 at 14:28:28 UTC, Jens Mueller wrote:

 Can we have a generic function/code that is as efficient as yours?


 Well, for now, we have "map" (and "iota"). This usually covers enough
 ground, but there remains cases where it is sub-optimal.

 What we would need is a "generate". Basically, a range that calls a
 function to generate the value of front on the fly. I've seen enough
 threads like this one that "abuse" iota/repeat/take/map (or sequence and
 recurrence for that matter), just to obtain a generic generator.

 Let's just write Generator/generator.


Make sure you copy Sequence/sequence :o).

Andrei
Feb 23 2013
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 16:29:49 UTC, Andrei 
Alexandrescu wrote:

 On 2/23/13 5:56 PM, monarch_dodra wrote:

 On Saturday, 23 February 2013 at 14:28:28 UTC, Jens Mueller 
 wrote:

 Can we have a generic function/code that is as efficient as 
 yours?


 Well, for now, we have "map" (and "iota"). This usually covers 
 enough
 ground, but there remains cases where it is sub-optimal.

 What we would need is a "generate". Basically, a range that 
 calls a
 function to generate the value of front on the fly. I've seen 
 enough
 threads like this one that "abuse" iota/repeat/take/map (or 
 sequence and
 recurrence for that matter), just to obtain a generic 
 generator.

 Let's just write Generator/generator.


 Make sure you copy Sequence/sequence :o).

 Andrei


Have you actually tried it? Do you know what the signature of a 
function passed to sequence is?

I think this bug entry sums it up pretty well, and why "Sequence" 
is not really adapted:

http://forum.dlang.org/thread/bug-9550-3 http.d.puremagic.com%2Fissues%2F

//----
import std.stdio, std.range, std.conv, std.random, std.typecons;

void main()
{
     dstring letters = "abcd";
     dchar fun(Tuple!(), uint) {return letters[uniform(0, $)];}
     writeln(sequence!fun().take(9));
}
//----

I failed to use a lambda or mixin in this case.

I really don't think we should push to try to warp the existing 
stuff to work in any scenario, but rather, give the tools 
required to work intuitively for any scenario.
Feb 23 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/23/13 4:25 PM, Jens Mueller wrote:

 auto randomLetter = () =>  letters[uniform (0, letters.length)];
 auto randomString = iota(10).map!(_ =>  randomLetter()).array();


auto randomString = iota(10).map!(_ => letters[uniform(0, $)]).array;

Bearophile has long asked for a one-parameter uniform(n) that does what 
uniform(0, n) does. Perhaps this provides a good supporting argument. 
With that enhancement:

auto randomString = iota(10).map!(_ => letters[uniform($)]).array;

I expect this to be fast and only allocate once.


Andrei
Feb 23 2013
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 16:25:45 UTC, Andrei 
Alexandrescu wrote:

 On 2/23/13 4:25 PM, Jens Mueller wrote:

 auto randomLetter = () =>  letters[uniform (0, 
 letters.length)];
 auto randomString = iota(10).map!(_ =>  
 randomLetter()).array();


 auto randomString = iota(10).map!(_ => letters[uniform(0, 
 $)]).array;

 Bearophile has long asked for a one-parameter uniform(n) that 
 does what uniform(0, n) does. Perhaps this provides a good 
 supporting argument. With that enhancement:

 auto randomString = iota(10).map!(_ => 
 letters[uniform($)]).array;

 I expect this to be fast and only allocate once.


 Andrei


I'm still not a fan of the iota+map combo.

I wrote this (Fast)Generator template. The generic "generator" 
bookkeeps the calls to front and popFront.

The "fast" variant assumes that you'll basically just call 
front+popFront+front+popFront, or that you don't about bookeeping.

This can be interesting, as often, iota+map is straight up 
linearly consumed.

//----
import std.stdio, std.range, std.conv, std.random;

template zeroaryFun(alias fun)
{
     static if (is(typeof(fun) : string))
     {
         auto zeroaryFun()
         {
             mixin("return (" ~ fun ~ ");");
         }
     }
     else
     {
         alias fun zeroaryFun;
     }
}

struct FastGenerator(alias F)
{
     private alias fun = zeroaryFun!F;
     private alias T = typeof(fun());

     enum empty = false;
      property auto front() {return fun();}
     void popFront() {}
}
struct Generator(alias F)
{
     private alias fun = zeroaryFun!F;
     private alias T = typeof(fun());
     private T cache;
     private bool called = false;

     enum empty = false;

      property
     auto front()
     {
         if (!called)
         {
             cache = fun();
             called = true;
         }
         return cache;
     }

     void popFront()
     {
         if (!called)
             fun();
         called = false;
     }
}

void main()
{
     dstring letters = "abcd";
     int i = 0;
     writeln(FastGenerator!(() => (i++))().take(9)); //Who needs 
iota?
     writeln(FastGenerator!"'s'"().take(5)); //Who needs repeat?
     writeln(FastGenerator!(() => letters[uniform(0, 
$)])().take(9)); //Simple,clear,concise.
}
//----

In these cases, Generator is something that is both convenient 
and fast.

I realize it's not an incredibly higher order function or 
anything, but I wouldn't mind having something like this in 
Phobos. It would just make every day coding that much more 
convenient and simple.
Feb 23 2013
parent reply "deadalnix" <deadalnix gmail.com> writes:
Can't this do the trick ?

sequence!(_ => letters[uniform($)]).take(length).array() ?
Feb 23 2013
parent "bearophile" <bearophileHUGS lycos.com> writes:
deadalnix:


 Can't this do the trick ?

 sequence!(_ => letters[uniform($)]).take(length).array() ?


uniform() requires two arguments.

Bye,
bearophile
Feb 23 2013
prev sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Andrei Alexandrescu:


 Bearophile has long asked for a one-parameter uniform(n) that 
 does what uniform(0, n) does.


I think that's false (you are maybe confused with my old request 
for iota(n)).

I have asked for a choice():
http://d.puremagic.com/issues/show_bug.cgi?id=4851


And I have asked for a uniform01() that does what uniform(0.0, 
1.0) does. It's a commonly useful need, and it can be generated 
faster than the generic uniform(a, b):
http://d.puremagic.com/issues/show_bug.cgi?id=5240

Bye,
bearophile
Feb 23 2013
prev sibling parent reply "jerro" <a a.com> writes:
     string randomString;
     {
         auto tmp = new char[](10);
         foreach(ref char c ; tmp)
             c = letters[uniform(0, letters.length)];
         randomString = cast(string)letters;
     }

 All those scope blocks I put in are not mandatory. At the end 
 of the day, it is 4 lines of code. Very efficient, and the 
 intent is crystal clear.


Or you could simply do this:

string randomString = iota(10).map!(_ => letters[uniform(0, 
$)]).array;

It's one line off code and the intent seems crystal clear to me. 
It's also equally efficient. I've benchmark those to pieces of 
code for various values of n with both DMD and GDC, using -O 
-inline -release:

auto tmp = iota(n).map!(_ => letters[uniform(0, $)]).array;

....

auto tmp = new char[](n);
foreach(ref char c ; tmp)
     c = letters[uniform(0, letters.length)];

For n=10 and when compiled with GDC, the first snippet actually 
performed a little bit (about 10%) better, but for larger sizes 
there was no significant difference.
Feb 23 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 19:08:04 UTC, jerro wrote:

    string randomString;
    {
        auto tmp = new char[](10);
        foreach(ref char c ; tmp)
            c = letters[uniform(0, letters.length)];
        randomString = cast(string)letters;
    }

 All those scope blocks I put in are not mandatory. At the end 
 of the day, it is 4 lines of code. Very efficient, and the 
 intent is crystal clear.


 Or you could simply do this:

 string randomString = iota(10).map!(_ => letters[uniform(0, 
 $)]).array;

 It's one line off code and the intent seems crystal clear to 
 me. It's also equally efficient. I've benchmark those to pieces 
 of code for various values of n with both DMD and GDC, using -O 
 -inline -release:

 auto tmp = iota(n).map!(_ => letters[uniform(0, $)]).array;

 ....

 auto tmp = new char[](n);
 foreach(ref char c ; tmp)
     c = letters[uniform(0, letters.length)];

 For n=10 and when compiled with GDC, the first snippet actually 
 performed a little bit (about 10%) better, but for larger sizes 
 there was no significant difference.


In my defense, I had started writing that when the "1 liner" was 
still:

auto randomLetter = () => randomSample(letters, 1, 
letters.length).front;
writeln(iota(10).map!(_ => randomLetter()));

The "new" 1-liner is indeed good, and what I would actually use. 
I still think though that there is a point where you need to stop 
and think "is my 1 liner actually understandable and 
maintainable". In this first case, the answer (IMO), was no.

Now it is, so good for us.

BTW, I think the clearest remains my generator proposal:
string randomString =
     fastGenerator!(() => letters[uniform(0, $)]).take(9).array;

Any chance you could tell me how it fares in your bench?
Feb 23 2013
parent reply "jerro" <a a.com> writes:
 BTW, I think the clearest remains my generator proposal:
 string randomString =
     fastGenerator!(() => letters[uniform(0, $)]).take(9).array;


If the goal was to replace iota(n).map, it may be better to just 
have something like:

     generate!(() => letters[uniform(0, $)])(n).array

But of course, your fastGenerator is more general. I personally 
don't often have a need to generate an infinite range in this 
way, but other people may.


 Any chance you could tell me how it fares in your bench?


I used this code:

auto tmp = FastGenerator!(() => letters[uniform(0, 
$)])().take(n).array;

When I build it with GDC, it performs about the same as the code 
that uses iota and map. When I build it with DMD it's about 10% 
slower than the code that uses iota and map. It seems that DMD 
fails to inline the lambda in this case.

By the way, this is not a very good benchmark for ranges because 
most of the time is spent generating random numbers.
Feb 23 2013
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Saturday, 23 February 2013 at 20:21:18 UTC, jerro wrote:

 BTW, I think the clearest remains my generator proposal:
 string randomString =
    fastGenerator!(() => letters[uniform(0, $)]).take(9).array;


 If the goal was to replace iota(n).map, it may be better to 
 just have something like:

     generate!(() => letters[uniform(0, $)])(n).array


Arguably, the goal is to actually replace "repeat+map". I view 
the fact that iota not being infinite as a drawback. And even if 
you do want a bound to the length of your range, I believe it 
should be the "top" step in your types, as opposed to the 
"bottom" step. Eg:

iota(n).map!(_ => letters[uniform(0, $)]).array;
vs
repeat(0).map!(_ => letters[uniform(0, $)]).take(n).array;

*Personally*, I prefer the logic behind the repeat+map+take, over 
iota+map's. It might actually even be faster. It is slightly more 
verbose though :/

------
Another advantage "generate" would have over repeat+map is if you 
want to pass an actual named impure function (as opposed to 
lambda):

iota(0, n).map(_ => fun());
or
repeat(0).map(_ => fun()).take(n);
vs
fastGenerate!fun().take(n);

Here, the generate would be much more "idiomatic", and also 
probably easier on the compile (function vs lamba: easier 
inlining, as you mention below).


 But of course, your fastGenerator is more general. I personally 
 don't often have a need to generate an infinite range in this 
 way, but other people may.


 Any chance you could tell me how it fares in your bench?


 I used this code:

 auto tmp = FastGenerator!(() => letters[uniform(0, 
 $)])().take(n).array;

 When I build it with GDC, it performs about the same as the 
 code that uses iota and map. When I build it with DMD it's 
 about 10% slower than the code that uses iota and map. It seems 
 that DMD fails to inline the lambda in this case.

 By the way, this is not a very good benchmark for ranges 
 because most of the time is spent generating random numbers.


Another thing I'd like to re-bring up (which I've partially 
implemented already), was bearophile's pass/tee suggestion:
http://forum.dlang.org/thread/zkioveywsgxiovzvdypq forum.dlang.org

It's those little functions that don't seem like much at first, 
but that really add up to a language's expressiveness (IMO).
Feb 23 2013
parent "jerro" <a a.com> writes:
 Arguably, the goal is to actually replace "repeat+map". I view 
 the fact that iota not being infinite as a drawback. And even 
 if you do want a bound to the length of your range, I believe 
 it should be the "top" step in your types, as opposed to the 
 "bottom" step. Eg:

 iota(n).map!(_ => letters[uniform(0, $)]).array;
 vs
 repeat(0).map!(_ => letters[uniform(0, $)]).take(n).array;

 *Personally*, I prefer the logic behind the repeat+map+take, 
 over iota+map's. It might actually even be faster. It is 
 slightly more verbose though :/


There is a way to have an infinite generate range and also a 
convenient way to initialize arrays - just add an overload of 
array() that takes the length as the second parameter. Then we 
can do:

auto randomString = generate!(() => letters[uniform(0, 
$)]).array(10);


 ------
 Another advantage "generate" would have over repeat+map is if 
 you want to pass an actual named impure function (as opposed to 
 lambda):

 iota(0, n).map(_ => fun());
 or
 repeat(0).map(_ => fun()).take(n);
 vs
 fastGenerate!fun().take(n);

 Here, the generate would be much more "idiomatic", and also 
 probably easier on the compile (function vs lamba: easier 
 inlining, as you mention below).


In general, named functions and lambdas are pretty much 
equivalent in regard to inlining (see http://goo.gl/XaOUP). In 
your example, the last line could  be more efficient than the 
first two, because in the first two lines there could be one 
extra function call in the worst case.


 Another thing I'd like to re-bring up (which I've partially 
 implemented already), was bearophile's pass/tee suggestion:
 http://forum.dlang.org/thread/zkioveywsgxiovzvdypq forum.dlang.org


I agree this is a good idea.
Feb 23 2013
prev sibling parent Iain Buclaw <ibuclaw ubuntu.com> writes:
On 23 Feb 2013 20:25, "jerro" <a a.com> wrote:

 BTW, I think the clearest remains my generator proposal:
 string randomString =
     fastGenerator!(() => letters[uniform(0, $)]).take(9).array;



 If the goal was to replace iota(n).map, it may be better to just have


something like:

     generate!(() => letters[uniform(0, $)])(n).array

 But of course, your fastGenerator is more general. I personally don't


often have a need to generate an infinite range in this way, but other
people may.

 Any chance you could tell me how it fares in your bench?



 I used this code:

 auto tmp = FastGenerator!(() => letters[uniform(0, $)])().take(n).array;

 When I build it with GDC, it performs about the same as the code that


uses iota and map. When I build it with DMD it's about 10% slower than the
code that uses iota and map. It seems that DMD fails to inline the lambda
in this case.


That is deliberate on gdc's part to mark all lambdas as inlineable as most
just do one computation and don't require any custom static chain built to
access locals in parent functions.  :)

Regards
----
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';



 Feb 24 2013