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

User option to set type parameter of LogBinner #6

Open
carstenbauer opened this issue Jan 29, 2019 · 0 comments
Open

User option to set type parameter of LogBinner #6

carstenbauer opened this issue Jan 29, 2019 · 0 comments
Labels
enhancement New feature or request

Comments

@carstenbauer
Copy link
Owner

Currently we use a heuristic to select an appropriate type. In case this isn't what the user wants, he should be able to set the type manually.

Heuristic:
"If I get a type <: Real then make x_sum etc. take Float64 and if I get <: Complex then make x_sum store ComplexF64 values. Although not strictly valid it should handle most practical cases."

Why is it necessary? (taken from old issue)

Try creating a binner with zero(Int64) and add more than one element. This will fail because the averaging of the first compressor, 0.5 * (C.value + value), will produce a Float64 and there is no method push! for Float64 for this binner.

Maybe check in push! whether one is pushing to the first level. If yes, only allow values of type T, otherwise allow any type. We should be the only ones pushing to higher levels anyway, so probably we should rename push! (with a level argument) to _push! to avoid exporting it.

However, this won't solve it fully, as the problem propagates: from level 2 on, x_sum and x2_sum will get Float64s which they can't handle as they have eltype(x_sum) == Int64.

@carstenbauer carstenbauer added the enhancement New feature or request label Jan 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant