JuliaMath / JuliaMath/Interpolations.jl

RFC: possible rework of element type and inferrability

Open
#578 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
Julia
Stars
575
Forks
117
PR merge metrics
No merged PRs in 30d

Description

I've seen cases that make me think that we may need to redesign how construction and position-evaluation compute types. To be clear, I suspect that I am responsible for the current situation, so any weaknesses are my fault. This interacts quite closely with #242 and perhaps it would be best to address both together. See also #359.

There is some ambiguity about what "element type" means for a container like an AbstractInterpolation. At one point, I think the clearest statement came from Jeff Bezanson, which is that the eltype should be the type of object returned when evaluated at integer positions, much as how `getindex` works on `AbstractArrays`. However, evaluating at other positions (or with positions encoded by non-integer types) may return objects of different type, because the mathematics performed may be different. One strong possibility is that if we implement #242, then we should make `eltype(::AbstractInterpolation)` a `MethodError`.

I've put together a short, simple test that illustrates how I think it should work. Currently, nearly half of these fail.

```julia
using Interpolations
using Test

# Non-promotable types that nevertheless have all required arithmetic operations defined.
# Except for the lack of promotion, CoefType ≈ Int and MultType ≈ Float64.
struct CoefType end
struct MultType end
for T in (CoefType, MultType)
@eval begin
Base.:*(::$T, ::$T) = MultType()
Base.:*(::Real, ::$T) = MultType()
Base.:*(::$T, ::Real) = MultType()
end
end
Base.:*(::CoefType, ::Int) = CoefType()
Base.:*(::Int, ::CoefType) = CoefType()
Base.:*(::CoefType, ::Int) = CoefType()

Base.:+(::MultType, ::MultType) = MultType()
Base.:+(::CoefType, ::CoefType) = CoefType()

@testset "Non-promotable types" begin
@test promote_type(CoefType, MultType) === Any # Any is the only type that "contains" both

a = fill(CoefType(), 3)
itp = interpolate(a, BSpline(Linear()))
# When indexed with integers, linear interpolation does not perform any arithmetic operations.
# Therefore, the result is of type CoefType.
@test eltype(itp) === CoefType # or a MethodError? But if it doesn't error, I think this is what it should return (for Linear)
@test @inferred(itp(1)) === CoefType()
# But when evaluated at intermediate positions, linear interpolation does perform arithmetic operations.
# Therefore, the result is of type MultType.
@test @inferred(itp(1.5)) === MultType()

# Extrapolation should behave similarly.
etp = extrapolate(itp, CoefType())
@test eltype(etp) === CoefType
@test @inferred(etp(1)) === CoefType()
@test @inferred(etp(1.5)) === MultType()
# Ensure that beyond-the-edge works the same as within-the-edge.
@test @inferred(etp(4)) === CoefType()
@test @inferred(etp(4.5)) === MultType()
end
```

#### Take-aways

We shouldn't use `promote` or `promote_type`. Fundamentally, promotion is about *storage* ("how do I create a container that can hold both of these objects?"), and that's different from computing the type of object returned by, e.g., `(1-f)*a + f*b`.

Downstream users, however, may receive new bugs from the lack of assumed real-arithmetic "widening". That is, currently we have
```julia
julia> eltype(interpolate([1, 2, 3], BSpline(Linear())))
Float64
```
because we effectively assume that the created object will be evaluated at `Float64`-encoded positions. If we change that to `Int`, as I'm proposing here (because the coefficients are `Int`), I can imagine that will break a lot of downstream code that relies on `eltype(itp)` returning something that can accomodate both `Int` and `AbstractFloat` evaluation locations. Making `eltype` throw a `MethodError` would be one way of preventing more subtle bugs because of this change.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.