search

haskell-numerics / random-fu

A suite of Haskell libraries for representing, manipulating, and sampling random variables
42 stars 21 forks source link

weightedCategorical (weightedCategoricalT) documentation is identical to categorical (categoricalT) #33

Closed nsmryan closed 7 years ago

nsmryan commented 8 years ago

The documentation string for weightedCategorical says that the weights must sum to 1. I believe that this is a copy paste issue, since weightedCategorical seems to normalize the weights, and would otherwise be identical to categorical. This applies to weightedCategoricalT as well.

idontgetoutmuch commented 8 years ago

Thanks for this. I will take a look over the next few days.

dmwit commented 8 years ago

I have just noticed the same thing. From source diving, I think the description is anyway wrong for both -- neither weightedCategorical nor categorical actually seem to assume that the weights sum to 1. (For both, during sampling, a random number between 0 and the sum of all the weights is chosen uniformly, whatever that sum may be.) The difference between the two appears to be that one removes zero-probability events and the other doesn't; the re-weighting so that weights sum to 1 may be worth mentioning but should only matter if the underlying probability type's uniform distribution doesn't respect scaling.

UnkindPartition commented 7 years ago

Hi, Dominic, and happy new year!

I just stumbled upon this documentation issue, too (and probably not the first time...)

Could you fix it up?

idontgetoutmuch commented 7 years ago

I am looking now...

idontgetoutmuch commented 7 years ago

Ok I pushed a documentation change.

BTW I am not totally convinced of performance here. I haven't investigated the existing code but it can be quicker to use the inverse CDF with a uniform sampler and it's not obvious to me that this is being used in the code.