Hello Alvaro & Tom,
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> Seems that in the review so far, Fabien has focused mainly in the
>> mathematical properties of the new random number generation. That seems
>> perfectly fine, but no comment has been made about the chosen UI for the
>> feature.
>> Per the few initial messages in the thread, in the patch as submitted
>> you ask for a gaussian random number by using \setgaussian, and
>> exponential via \setexp. Is this the right UI?
I thought it would be both concise & clear to have that as another form of
\set*.
If I had it designed from the start, I think I may have put only "\set"
with some functions such as "uniform", "gaussian" and so on. but once
there is a set and a setrandom for uniform, this suggested other settings
would have their own set commands as well. Also, the number of expected
arguments is not the same, so it may make the parsing code less obvious.
Finally, this is not a "language" heavily used, so I would emphasize
simpler code over more elegant features, for once.
>> Currently you get an evenly distributed number with \setrandom. There
>> is nothing that makes it obvious on \setgaussian by itself that it
>> produces random numbers.
Well, "gaussian" or "exp" are kind of a clue, at least to my
mathematically-oriented mind.
>> Perhaps we should simply add a new argument to \setrandom, instead of
>> creating new commands for each distribution? I would guess that, in
>> the future, we're going to want other distributions as well.
>
> +1 for an argument to \setrandom instead of separate commands.
>> Not sure what it would look like; perhaps
>> \setrandom foo 1 10 gaussian
There is an additional argument expected. That would make:
\setrandom foo 1 10 [uniform] \setrandom foo 1 :size gaussian 3.6 \setrandom foo 1 100 exponential 7.2
> FWIW, I think this style is sufficient; the others seem overcomplicated
> for not much gain. I'm not strongly attached to that position though.
If there is a change, I agree that one simple style is enough, especially
as the parsing code is rather low-level already.
So I'm basically fine with the current status of the patch, but I would
be okay with a \setrandom as well.
--
Fabien.