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

adapt_structure for StepRangeLen does not preserve eltype #87

Closed
glwagner opened this issue Oct 30, 2024 · 1 comment · Fixed by #88
Closed

adapt_structure for StepRangeLen does not preserve eltype #87

glwagner opened this issue Oct 30, 2024 · 1 comment · Fixed by #88

Comments

@glwagner
Copy link
Contributor

adapt_structure for StepRangeLen,

Adapt.jl/src/base.jl

Lines 79 to 80 in 5ef7c53

adapt_structure(to, r::StepRangeLen) =
StepRangeLen(adapt(to, r.ref), adapt(to, r.step), r.len, r.offset)

does not preserve the eltype of the range. For example:

julia> r = range(Float32(1), Float32(2), length=3); typeof(r)
StepRangeLen{Float32, Float64, Float64, Int64}

julia> using Adapt

julia> r_adapted = Adapt.adapt_structure(nothing, r); typeof(r_adapted)
StepRangeLen{Float64, Float64, Float64, Int64}

It's because the constructor called by adapt_structure has a different rule for working out the eltype than range, I suppose...

https://github.com/JuliaLang/julia/blob/9850a3881221a57a382e98c9b9ae2bf97ac3966d/base/range.jl#L524-L525

It's not a problem for Float64 ranges.

I think it's an easy fix by using the constructor StepRangeLen{T}(r.ref, r.step, r.len, r.offset).

@glwagner
Copy link
Contributor Author

cc @ali-ramadhan

x-ref CliMA/Oceananigans.jl#3870

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