-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Converting StaticInt
to a different type causes invalidations
#52
Comments
The irritating thing here is that if we remove the definition, we get a |
Would that just be a PR with a bunch of |
I opened up a draft PR for this. I welcome any ideas to get this issue resolved. |
maybe require |
If we remove these two it fixes it: Base.convert(::Type{T}, @nospecialize(N::StaticInt)) where {T<:Number} = convert(T, Int(N))
(::Type{T})(@nospecialize(x::StaticInt)) where {T<:Integer} = T(known(x)) but it also leads to a lot of other stuff not working anymore. We also would benefit from this PR. |
@Tokazama Mind making a PR with a breaking version bump that gets rid of all the invalidations? We can have an ugly API, but I think under the view that We should be willing to optimize libraries for performance (load and run time), even if it's at the expense of beauty. But explicit/easy to read isn't necessarily ugly/has its own advantages for maintainability. |
Sure I can do that, but as a heads up it's going to be a bit of a mess for a while. It's essentially the same as removing the |
Static.jl/src/int.jl
Line 26 in 747cc66
This definition causes conversion of the types of
(::Type{T})(::Integer)
to be invalidated becauseStaticInt <: Integer
. Since Flux uses ArrayInterface, it pulls in Static and shows up the invalidations.List of invalidations as seen in a session with Flux, but most of the methods seem be in Compiler, Base and StdLibs
The text was updated successfully, but these errors were encountered: