Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about rounding #154

Open
GitCaptain opened this issue Feb 14, 2022 · 1 comment
Open

Question about rounding #154

GitCaptain opened this issue Feb 14, 2022 · 1 comment

Comments

@GitCaptain
Copy link

Hi, @jansel, I've been looking through your manipulator code, and stuck on these lines:

if self.is_integer_type():
# account for rounding
low -= 0.4999
high += 0.4999

Could you please explain this strange rounding?
What is 0.4999 const (why not 0.5)?
Also, isn't it possible that we will go beyond the allowed range (because we rounding low to the floor and high to the ceil)?
And if we check, that low and high are integrals (self.is_integer_type), why is this rounding needed at all?

@jansel
Copy link
Owner

jansel commented Feb 15, 2022

get_unit_value()/set_unit_value() returns a float value between 0.0 and 1.0. This allows some techniques to work on only floats and treat integers as continuous.

Suppose you have an integer from 0 to 4.

With the offsets:

  • 0.0 to 0.2 maps to 0
  • 0.2 to 0.4 maps to 1
  • 0.4 to 0.6 maps to 2
  • 0.6 to 0.8 maps to 3
  • 0.8 to 1.0 maps to 4

Without the offsets:

  • 0.000 to 0.125 maps to 0
  • 0.125 to 0.375 maps to 1
  • 0.375 to 0.625 maps to 2
  • 0.625 to 0.875 maps to 3
  • 0.875 to 1.000 maps to 4

The no-offset version has a biased distribution, where 0 and 4 are less likely to be chosen as they occupy a smaller part of the search space. This is due to rounding, you can never pick 4.4 and round that to 4, but you can pick 3.4 and round that to 3.

I think it would be fine to change these to 0.5 (and perhaps more correct). I believe my original thinking (from years ago) was to make sure the rounding on line 502 happened in the right direction in set_unit_value, but it looks like line 503 will handle that case anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants