|
|
|
Created:
16 years, 1 month ago by wjosephson Modified:
8 years, 4 months ago Reviewers:
rsc CC:
rsc Visibility:
Public. |
Zipf distributed random variates.
Patch Set 1 #
Total comments: 6
Patch Set 2 : code review 176070: Zipf distributed random variates. #Patch Set 3 : code review 176070: Zipf distributed random variates. #
Total messages: 4
|
rsc
looks pretty good. a few minor nits. http://codereview.appspot.com/176070/diff/1/3 File src/pkg/rand/zipf.go (right): http://codereview.appspot.com/176070/diff/1/3#newcode14 src/pkg/rand/zipf.go:14: // A ...
|
16 years, 1 month ago (2009年12月14日 03:13:10 UTC) #1 | ||||||||||||||||||||||
looks pretty good. a few minor nits. http://codereview.appspot.com/176070/diff/1/3 File src/pkg/rand/zipf.go (right): http://codereview.appspot.com/176070/diff/1/3#newcode14 src/pkg/rand/zipf.go:14: // A Zipf generates Zipf distributed variates period at end of sentence http://codereview.appspot.com/176070/diff/1/3#newcode52 src/pkg/rand/zipf.go:52: // Initialize a Zipf so that variates p(k) are generated on [0, imax] should be a sentence. NewZipf returns a Zipf generating p(k) on [0, imax] proportional to (v+k)**(-s) where s>1, k>=0, and v>=1. (I added the v >= 1.) http://codereview.appspot.com/176070/diff/1/3#newcode57 src/pkg/rand/zipf.go:57: if v == 0 { So v == 0 is okay but other v < 1 is not? Seems cleaner to require v >= 1 and drop this, but I don't know what the usages are like. http://codereview.appspot.com/176070/diff/1/3#newcode60 src/pkg/rand/zipf.go:60: if z.init(r, s, v, imax) { might as well inline the init call since it is not used elsewhere. http://codereview.appspot.com/176070/diff/1/3#newcode69 src/pkg/rand/zipf.go:69: var k = float64(0.0); s/var k =/k :=/ http://codereview.appspot.com/176070/diff/1/3#newcode78 src/pkg/rand/zipf.go:78: } else if ur >= z.h(k+0.5)-math.Exp(-math.Log(k+z.v)*z.q) { no need for else since the if broke. } if ur >= ... (or use || above)
looks good to me; thanks.
LGTM When you hg sync you're going to get a merge conflict because I've stripped the semicolons with hg gofmt. You'll be able to resolve it with cd $GOROOT/src/pkg/rand hg revert zipf.go hg resolve -m zipf.go # marks it resolved
*** Submitted as http://code.google.com/p/go/source/detail?r=230b42431917 *** rand: Zipf distributed random variates. R=rsc http://codereview.appspot.com/176070 Committer: Russ Cox <rsc@golang.org>