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

Return type of entropy changes with the base #924

Closed
sgaure opened this issue Apr 3, 2024 · 1 comment · Fixed by #925
Closed

Return type of entropy changes with the base #924

sgaure opened this issue Apr 3, 2024 · 1 comment · Fixed by #925

Comments

@sgaure
Copy link

sgaure commented Apr 3, 2024

I came across a somewhat unexpected property which I'm uncertain what I think of.

p = rand(Float32, 10)
p ./= sum(p)
typeof(entropy(p))    # Float32
typeof(entropy(p, 2))  # Float64
typeof(entropy(p, MathConstants.e)) # Float32

When specifying a base b, the result is "scaled by 1/log(b)" according to the docs, which is almost true. It is divided by log(b), which explains the last example. log(MathConstants.e) is an Int64, and dividing a Float32 by an Int64 yields a Float32.

This isn't a big problem, everything is type stable, but it's unexpected if one otherwise operates on Float32 (or Float16 or something else). It's of course easy to make it return Float32 in the second case by letting b be 2f0.

Anyway, should the return type of entropy depend on the type of the base b at all? Shouldn't it have the element type of p? Or the same type as the default base e entropy? I'm not suggesting a change right away, because it could possibly break existing software. But it should be considered.

@sgaure
Copy link
Author

sgaure commented Apr 3, 2024

On second thought, it could be someone did something like this:

import ForwardDiff
ForwardDiff.derivative(x -> entropy(p, x), 2)

which would break if entropy(p,x) returns the type of entropy(p)

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

Successfully merging a pull request may close this issue.

1 participant