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

brief.jl:There is an error in the range #68

Open
zhuxyu opened this issue Nov 17, 2019 · 3 comments
Open

brief.jl:There is an error in the range #68

zhuxyu opened this issue Nov 17, 2019 · 3 comments

Comments

@zhuxyu
Copy link

zhuxyu commented Nov 17, 2019

I think there was an error in brief.jl, the default value range of rand() is [0,1), so I think in the random_unfiorm function, rand() should be changed to (rand().-0.5) * 2

julia> brief_params = BRIEF(sampling_type = random_uniform)
BRIEF{typeof(random_uniform)}(128, 9, 1.4142135623730951, ImageFeatures.random_uniform, 123)
julia> brief_params.window      # window = 9, so the value of CartesianIndex should in [-4:4]
9
julia> B = brief_params.sampling_type(brief_params.size, brief_params.window, brief_params.seed)     # All values of the index are positive and in [0:4].
(CartesianIndex{2}[CartesianIndex(4, 3), CartesianIndex(2, 1), CartesianIndex(0, 1), CartesianIndex(2, 3), CartesianIndex(2, 1), CartesianIndex(1, 3), CartesianIndex(4, 0), CartesianIndex(4, 2), CartesianIndex(4, 1), CartesianIndex(4, 4)    CartesianIndex(2, 0), CartesianIndex(2, 4), CartesianIndex(1, 1), CartesianIndex(3, 4), CartesianIndex(3, 1), CartesianIndex(4, 4), CartesianIndex(1, 1), CartesianIndex(2, 3), CartesianIndex(1, 0), CartesianIndex(3, 3)], CartesianIndex{2}[CartesianIndex(1, 3), CartesianIndex(0, 2), CartesianIndex(2, 0), CartesianIndex(2, 1), CartesianIndex(1, 2), CartesianIndex(3, 1), CartesianIndex(3, 2), CartesianIndex(3, 2), CartesianIndex(0, 2), CartesianIndex(1, 2)    CartesianIndex(1, 2), CartesianIndex(2, 0), CartesianIndex(2, 4), CartesianIndex(1, 4), CartesianIndex(0, 2), CartesianIndex(1, 0), CartesianIndex(0, 2), CartesianIndex(4, 3), CartesianIndex(3, 1), CartesianIndex(0, 1)])
@Jay-sanjay
Copy link

so what it is showing after changing to (rand().-0.5) * 2

@Jay-sanjay
Copy link

(rand()-0.5) * 2

@mkitti
Copy link
Member

mkitti commented Aug 7, 2023

This is the expression being considered.

        x_gen = floor(Int, window * rand() / 2)

Let's first consider the subexpression

window * rand() / 2

This will produce a pseudo random number in the range [0, window/2)

If window = 6, then the numbers will be picked fromba uniform distribution within [0,3).

Flooring this would result in the following mapping:

[0,1) -> 0
[1,2) -> 1
[2,3) -> 2

This should be essentially the same as rand(0:2)

If window = 7 then this would pick from a uniform random distribution from [0,3.5)

0,1) -> 0
[1,2) -> 1
[2,3) -> 2
[3,3.5) -> 3

This would be equivalent to rand([0,0,1,1,2,2,3]) as 3 is selected half as frequently as the other integers.

The proposal is to use (rand().-0.5) * 2 which should be equal to 2rand() - 1.0. This draws a random number from [-1,1).

If window = 6, then values [-3,3) would be sampled with the equivalent being rand(-3:2).

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

3 participants