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

Use static numbers as element type of UnitWeights? #783

Open
oschulz opened this issue Apr 6, 2022 · 2 comments
Open

Use static numbers as element type of UnitWeights? #783

oschulz opened this issue Apr 6, 2022 · 2 comments

Comments

@oschulz
Copy link
Contributor

oschulz commented Apr 6, 2022

Currently, element access of UnitWeights loses the information that the weight is static, since uweights(5)[1] isa Int. Having uweights(5)[1] isa Static.StaticInt could power compiler optimizations in generic code that deals with weights.

Static.jl would be a lightweight dependency on top of StatsBase.jl so load-time impact would likely be very small:

julia> @time using StatsBase
  0.408891 seconds (839.31 k allocations: 47.544 MiB, 64.18% compilation time)

julia> @time using Static
  0.033636 seconds (96.28 k allocations: 5.571 MiB)

Sparked by SciML/Static.jl#51 .

@oschulz
Copy link
Contributor Author

oschulz commented Apr 6, 2022

CC @devmotion

@devmotion
Copy link
Member

Are indexing operations a common thing for UnitWeights? In my experience it's mainly used for dispatching to unweighted implementations, in which cases returning StaticInt seems less useful but potentially confusing or surprising. So at first glance I'm not completely convinced that it's a good idea - static numbers tend to be much less supported and it's more likely that users run into downstream issues.

Currently, also arbitrary number types (not only Int) are supported as elements of UnitWeights which would not be possible anymore. BTW due to this general definition it is already possible to do

julia> using StatsBase, Static

julia> u = uweights(StaticInt, 5);

julia> u[1] isa StaticInt
true

julia> u[1]
static(1)

IMO this makes it even less useful to change the current definition.

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