Implement getspace only for instances #993
Conversation
|
|
||
| @inline floatof(::Type{T}) where {T <: Real} = typeof(one(T)/one(T)) | ||
| @inline floatof(::Type) = Real |
There was a problem hiding this comment.
This method is needed for type unstable code and when type inference fails. The input to this comes from Core.Compiler.return_type which returns Any in some cases.
|
Thanks for the PR. Looks good to me, other than the one comment above. |
devmotion
changed the title
src/inference/Inference.jl
Outdated
| @inline floatof(::Type{T}) where {T <: Real} = typeof(one(T)/one(T)) | ||
| @inline floatof(::Type) = Real | ||
| floatof(::Type{T}) where {T <: Real} = typeof(one(T)/one(T)) | ||
| floatof(::Type{Any}) = Real # fallback if type inference failed |
There was a problem hiding this comment.
I would just get rid of Any here. For example, Core.Compiler.return_type can also return Union{} sometimes. Number may also not be impossible to get as input here.
There was a problem hiding this comment.
Ah, I somehow assumed it would always return Any if type inference fails.
|
I don't see how the HMC reverse diff test error would be related to this PR. |
|
Ya it's probably just a tolerance thing. Please increase the number of samples in that test and see if it passes. |
|
Maybe we should just fix a seed? The test passes for all other OS and Julia versions. Probably that should be done for the distribution test that fails from time to time as well. |
According to the Julia documentation, one should implement methods only for either instances or types, if possibly. This PR removes unneeded implementations of
getspacefor types.