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

promote FixedPointNumber to float when necessary #413

Open
johnnychen94 opened this issue Apr 10, 2021 · 2 comments
Open

promote FixedPointNumber to float when necessary #413

johnnychen94 opened this issue Apr 10, 2021 · 2 comments

Comments

@johnnychen94
Copy link
Contributor

JuliaImages widely uses FixedPointNumbers.N0f8, which is internally an UInt8, to represent [0,1] range. It seems that interpolate fails to promote the datatype for some interpolation types:

using FixedPointNumbers, Interpolations

interpolate(rand(N0f8, 4, 4),  BSpline(Quadratic(Flat(OnGrid()))))
# ERROR: ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent 1.1045751633986929

float(N0f8) # Float32
error stack
julia> itp = interpolate(rand(N0f8, 4, 4),  BSpline(Quadratic(Flat(OnGrid()))))
ERROR: ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent 1.1045751633986929
Stacktrace:
  [1] throw_converterror(#unused#::Type{N0f8}, x::Float64)
    @ FixedPointNumbers ~/.julia/packages/FixedPointNumbers/HAGk2/src/FixedPointNumbers.jl:326
  [2] _convert
    @ ~/.julia/packages/FixedPointNumbers/HAGk2/src/normed.jl:76 [inlined]
  [3] FixedPoint
    @ ~/.julia/packages/FixedPointNumbers/HAGk2/src/FixedPointNumbers.jl:58 [inlined]
  [4] convert
    @ ./number.jl:7 [inlined]
  [5] setindex!
    @ ./array.jl:841 [inlined]
  [6] setindex!
    @ ./multidimensional.jl:639 [inlined]
  [7] _A_ldiv_B_md!(dest::Matrix{N0f8}, F::LinearAlgebra.LU{Float64, LinearAlgebra.Tridiagonal{Float64, Vector{Float64}}}, src::Matrix{N0f8}, R1::CartesianIndices{0, Tuple{}}, R2::CartesianIndices{1, Tuple{Base.OneTo{Int64}}}, b::Vector{N0f8})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/filter1d.jl:39
  [8] _A_ldiv_B_md!(dest::Matrix{N0f8}, W::WoodburyMatrices.Woodbury{Float64, LinearAlgebra.LU{Float64, LinearAlgebra.Tridiagonal{Float64, Vector{Float64}}}, SparseArrays.SparseMatrixCSC{Float64, Int64}, SparseArrays.SparseMatrixCSC{Float64, Int64}, Matrix{Float64}, Matrix{Float64}}, src::Matrix{N0f8}, R1::CartesianIndices{0, Tuple{}}, R2::CartesianIndices{1, Tuple{Base.OneTo{Int64}}}, b::Vector{N0f8})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/filter1d.jl:78
  [9] A_ldiv_B_md!(dest::Matrix{N0f8}, F::WoodburyMatrices.Woodbury{Float64, LinearAlgebra.LU{Float64, LinearAlgebra.Tridiagonal{Float64, Vector{Float64}}}, SparseArrays.SparseMatrixCSC{Float64, Int64}, SparseArrays.SparseMatrixCSC{Float64, Int64}, Matrix{Float64}, Matrix{Float64}}, src::Matrix{N0f8}, dim::Int64, b::Vector{N0f8})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/filter1d.jl:17
 [10] prefilter!
    @ ~/.julia/packages/Interpolations/qHlUr/src/b-splines/prefiltering.jl:59 [inlined]
 [11] prefilter(#unused#::Type{Float64}, #unused#::Type{N0f8}, A::Base.ReinterpretArray{N0f8, 2, UInt8, Matrix{UInt8}, false}, it::BSpline{Quadratic{Flat{OnGrid}}})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/b-splines/prefiltering.jl:40
 [12] interpolate(#unused#::Type{Float64}, #unused#::Type{N0f8}, A::Base.ReinterpretArray{N0f8, 2, UInt8, Matrix{UInt8}, false}, it::BSpline{Quadratic{Flat{OnGrid}}})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/b-splines/b-splines.jl:160
 [13] interpolate(A::Base.ReinterpretArray{N0f8, 2, UInt8, Matrix{UInt8}, false}, it::BSpline{Quadratic{Flat{OnGrid}}})
    @ Interpolations ~/.julia/packages/Interpolations/qHlUr/src/b-splines/b-splines.jl:184
 [14] top-level scope
    @ REPL[3]:1
@mkitti
Copy link
Collaborator

mkitti commented Apr 10, 2021

I think we have two separate issues here:

  1. BSpline interpolation has no guarantee the interpolated values will be within the bounds of the numbers it is interpolating between. The interpolated value could exceed the maximum value, 1.0, as happens in your specific example. This is not fixable here. If you want to allow for overflow and then subsequent truncation, the user should be rather explicit about that by manually converting to floating point. Your specific example shows this case. BSpline interpolation is simply not appropriate for arbitrary integer data.

  2. The second issue is a type promotion issue between Ratio.SimpleRatio and N0f8, neither of which is defined in Interpolations.

julia> r = reinterpret(N0f8,[0x73, 0x94, 0x66, 0x1e])
4-element reinterpret(N0f8, ::Vector{UInt8}):
 0.451N0f8
 0.58N0f8
 0.4N0f8
 0.118N0f8

julia> interpolate(r,  BSpline(Quadratic(Flat(OnGrid()))))
4-element interpolate(OffsetArray(::Array{N0f8,1}, 0:5), BSpline(Quadratic(Flat(OnGrid())))) with element type Float64:
Error showing value of type Interpolations.BSplineInterpolation{Float64, 1, OffsetArrays.OffsetVector{N0f8, Vector{N0f8}}, BSpline{Quadratic{Flat{OnGrid}}}, Tuple{Base.OneTo{Int64}}}:
ERROR: promotion of types Ratios.SimpleRatio{Int64} and N0f8 failed to change any arguments

The direct fix belongs to Ratios which probably should fallback to promoting to a Rational for types not explicitly known to it like N0f8. The fix here in Interpolations.jl would involve limiting the use of SimpleRatio to specific input types. Because @timholy is still maintaining Ratios.jl as far as I know, let's see if he has any comment.

If you would like, open up an issue on Ratios.jl and I'll join you there.

@johnnychen94
Copy link
Contributor Author

If you want to allow for overflow and then subsequent truncation, the user should be rather explicit about that by manually converting to floating point.

Sounds valid to me. Feel free to close if you think there's nothing needs to do here.

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