Open JorySchossau opened 6 years ago
This issue is pretty interesting actually. On the one hand, we could have the 'current' behavior where the max value that readDouble can return is (upper bound - (1/aphabetSize)) which would make it act like typical random number generators for doubles (i.e. range is 0 to less than 1). But this is not intuitive. So the code is being changed so that if the site value is alphabetSize - 1 (i.e. the max allowed value) then the max value will be returned. This has no effect one readDouble when the type is double. Also, writeDouble for non-doubles needed to be updated so that (as much as possible) writing a value in a given range and reading the same site with the same range would result in the same value (accepting, rounding errors of course)
From a collaborator:
"One technicality I found that is a (very) minor bug is the following: This is from readDouble:
return ((value / genome->alphabetSize) * (valueMax - valueMin)) + valueMin;
Since the value can be between 0 and 255, in order to get probabilities including 0 and 1, one should divide by 255 instead of 256 (the genome->alphabetSize), if I am not mistaken. As is, the maximum possible probability is 255/256. Be careful that if you do it 1:256 divided by 256 you never get the 0. So it really should be 0:255 divided by 255. If I'm not totally mistaken. "