On Thu, Sep 17, 2015 at 7:56 PM, Henrik Nergaard <henrin10@student.uia.no> wrote:
-----Original Message----- From: Pharo-dev [mailto:pharo-dev-bounces@lists.pharo.org] On Behalf Of Ben Coman Sent: Wednesday, September 16, 2015 6:39 PM To: Pharo Development List <pharo-dev@lists.pharo.org> Subject: Re: [Pharo-dev] Pedantic on Random>>nextValue method name
On Wed, Sep 16, 2015 at 11:22 PM, Henrik Nergaard <henrin10@uia.no> wrote:
The method #nextValue actually calculates the next value the seed should be, the current value can be accessed by sending #seed.
I do agree that #nextValue might not be the clearest method name for its use; perhaps #nextSeed would be a better name?
Yes. Thats better again. Now its a private method, so there should be problem changing it ;) ??anyone
Continuing on another Random topic related to.. https://pharo.fogbugz.com/default.asp?16577
I notice that Collection>>atRandom is protected by a mutex but Collection>>atRandom: is not. This makes me believe the protection is for the Random generator and not for the actual picking of the element from the collection. So it seems that #atRandom holds the mutex too long - for both random generation and element picking. (> Is there a specific use for the mutex in this case? I tried to use a shared random without the mutex, and forking a few block closures, worked fine..
I was going to retort that inter-thread race conditions can be tricky to invoke with considerations of timing and priorities :P but then I had the insight this can be done using the debugger as demonstrated below. Presuming that use cases exist that require the repeatable nature of a PRNG. This feature is dependent on the /seed/ ivar having the same value in the calculation of both /hi/ and /lo/ in #nextValue. But consider this sequence of events... Priority 40 thread1 enters #next, then #nextValue, and takes the current value of /seed/ to calculate /hi/. Then before proceeding to the line, priority 50 thread2 enters #next and updates the value of /seed/. Thread1 then proceeds to take calculate /lo/ with a different value of /seed/ -- thus the algorithm is broken. This might be addressed by caching /seed/ ivar into a temporary /safeThreadCopySeed/. Yet other use cases may exist that expect simultaneous threads don't receive the same random number (more often than single thread would.) So priority 40 thread1 enters #next, then #nextValue, and copies the current value of /seed/ into /safeThreadCopySeed/. Then before it can update /seed/, priority 50 thread2 enters #next, then #nextValue, and copies the same current value of /seed/ into /safeThreadCopySeed/. So both threads will receive the same random number. To demonstrate... | sharedRandom context1 process1 thread1 result1 context2 process2 thread2 result2 | sharedRandom := Random new. context1 := [ result1 := sharedRandom next ] asContext. context2 := [ result2 := sharedRandom next ] asContext. process1 := Process forContext: context1 priority: 40. process2 := Process forContext: context2 priority: 40. thread1 := DebugSession process: process1 context: context1. thread2 := DebugSession process: process2 context: context2. 100 timesRepeat: [ result1 ifNil: [thread1 stepInto]. result2 ifNil: [thread2 stepInto]. ]. Transcript crShow: result1; tab; tab; show: result2.
If the mutex could be removed we could just add a method in the class side of Random like this:
globalGenerator ^Generator ifNil: [ Generator := Random new]
I don't think Random should have a global. Instead the same on the proposed SharedRandom might be suitable using a class-instance-variable... SharedRandom class instanceVariableNames: 'globalGenerator' SharedRandom>>globalGenerator ^ globalGenerator ifNil: [ globalGenerator := self new]. Then we can have... Collection>>atRandom ^ self randomAt: SharedRandom globalGenerator. cheers -ben
Thus I wonder if rather than copying the existing pattern of Collection>>atRandom to GlobalSharedRandom>>generateFor: it might be better to make a more generic SharedRandom as follows...
(> Maybe :P
I had a look at the Random Superclass in SciSmalltalk; here #next is used to create and give the next random value. #seed for seed, and #peek for checking what the next value would be without changing the current state. The peek method could be a good addition in Random -> Random>>#Peak Self nextValue / m
I would keep the way the #nextValue method behaves since this gives the option to "peek" at the next value the Ivar seed will have without changing it (could be useful?), but the name could be improved to be more clear. Perhaps the method name #nextSeed might not be so good after all, it might imply that the method creates a new seed, but what it actually does is to go to the next state in the random generation? ( so a better name could be #nextState ?), (perhaps the Ivar seed would also be better called state or internalState? )
<)
"Old calls to #next changed to private #nextValue"
Random>>nextValue | lo hi aLoRHi answer | hi := (seed quo: q) asFloat. lo := seed - (hi * q). " = seed rem: q" aLoRHi := (a * lo) - (r * hi). seed := (aLoRHi > 0.0) ifTrue: [aLoRHi] ifFalse: [aLoRHi + m]. ^ seed / m "this line moved here from old #next"
Random>>next ^ self nextValue
Random>>next: anInteger into: anArray 1 to: anInteger do: [:index | anArray at: index put: self nextValue]. ^ anArray
Random>>nextInt: anInteger anInteger strictlyPositive ifFalse: [ self error: 'Range must be positive' ]. anInteger asFloat isInfinite ifTrue: [^(self nextValue asFraction * anInteger) truncated + 1]. ^ (self nextValue * anInteger) truncated + 1
"SharedRandom protects all calls to private #nextValue"
Random subclass: SharedRandom instanceVariableNames: 'mutex'
SharedRandom>>initialize mutex := Semaphore forMutualExclusion.
SharedRandom>>next ^ mutex critical: [ self nextValue ]
SharedRandom>>next: anInteger into: anArray ^ mutex critical: [ super next: anInteger into: anArray ].
SharedRandom>>nextInt: anInteger ^ mutex critical: [ nextInt: anInteger ].
Then...
Object subclass: GlobalSharedRandom classVariables: 'sharedRandom'
GlobalSharedRandom class>>initialize sharedRandom := SharedRandom new.
GlobalSharedRandom class>>generator ^ sharedRandom
And finally...
Collection>>atRandom ^ self randomAt: GlobalSharedRandom generator. (> +1 <)