On 10/05/2019 14:27, Gilles Sadowski wrote:
Hi Alex.
Le ven. 10 mai 2019 à 13:57, Alex Herbert <alex.d.herb...@gmail.com> a écrit :
Can I get a review of the PR for RNG-101 please.
Thanks for this work!
I didn't go into the details; however, I see many fields and methods like
table1 ... table5
fillTable1 ... fillTable5
getTable1 ... getTable5
Wouldn't it be possible to use a 2D table:
table[5][];
so that e.g. only one "fillTable(int tableIndex, /* other args */)" method
is necessary (where "tableIndex" runs from 0 to 4)?
Yes. The design is based around using 5 tables as per the example code.
The sample() method knows which table it needs so it can directly jump
to the table in question. I'd have to look at the difference in speed
when using a 2D table as you are adding another array access but
reducing the number of possible method calls (although you still need a
method call). Maybe this will be optimised out by the JVM.
If the speed is not a factor then I'll rewrite it. Otherwise it's
probably better done for speed as this is the entire point of the
sampler given it disregards any probability under 2^-31 (i.e. it's not a
perfectly fair sampler).
Note that 5 tables are needed for 5 hex digits (base 2^6). The paper
states using 3 tables of base 2^10 then you get a speed increase
(roughly 1.16x) at the cost of storage (roughly 9x). Changing to 2
tables of base 2^15 does not make it much faster again.
I'll have a rethink to see if I can make the design work for different
base sizes.
The diff for "DiscreteSamplersList.java" refers to
MarsagliaTsangWangBinomialSampler
but
MarsagliaTsangWangSmallMeanPoissonSampler
seems to be missing.
Oops, I missed adding that back. I built the PR from code where I was
testing lots of implementations.
I've just added it back and it is still passing locally. Travis should
see that too as I pushed the change to the PR.
Regards,
Gilles
This is a new sampler based on the source code from the paper:
George Marsaglia, Wai Wan Tsang, Jingbo Wang (2004)
Fast Generation of Discrete Random Variables.
Journal of Statistical Software. Vol. 11, Issue. 3, pp. 1-11.
https://www.jstatsoft.org/article/view/v011i03
The code has no explicit licence.
The paper states:
"We have provided C versions of the two methods described here, for
inclusion in the “Browse
files”section of the journal. ... You may then want to examine the
components of the two files, for illumination
or for extracting portions that might be usefully applied to your
discrete distributions."
So I assuming that it can be incorporated with little modification.
The Java implementation has been rewritten to allow the storage to be
optimised for the required size. The generation of the tables has been
adapted appropriately and checks have been added on the input parameters
to ensure the sampler does not generate exceptions once constructed (I
found out the hard way that the original code was not entirely correct).
Thanks.
Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
