JuliaApproximation / DomainSets.jl

A Julia package for describing domains as continuous sets of elements
MIT License
72 stars 12 forks source link

ℤ contains non-integers #94

Closed cscherrer closed 2 years ago

cscherrer commented 2 years ago

Hi, for MeasureTheory we need to be able to represent subsets on the integers. But currently in DomainSets (v0.5.4),

julia> π ∈ ℤ
true

Can this be fixed?

daanhb commented 2 years ago

Heh, that does not look good :-)

It's good to understand why this happens, because there is a design decision underneath. Most domains have a type that inherits from Domain{T}. This is defined in IntervalSets.jl. But the T does not mean that all the elements of the domain have type T; instead the domain is defined by membership, i.e., whether or not x ∈ d returns true. For example, 0..1 is an Interval{Int64}, but 0.5 ∈ 0..1 is true because any other result would quickly lead to incomprehensible code. An Interval{Int64} contains all floating points in between the endpoints, even if the endpoints are integers.

DomainSets defines a FullSpace{T} which always returns true. At one point I thought it would be neat to define the real and integer numbers, but the looseness of T leads to what you found. This was clearly not a good idea.

I think the best way to proceed is to remove the current definitions of ℤ (and ℚ, ℝ, etcetera) from DomainSets. If they are good to have, one would have to make a specific domain for them, that does correctly defines their membership.

A sidenote here: it's good to bear in mind that 3..10 does not mean "all integers from 3 to 10". It means the (continuous) interval [3,10], regardless of the types of the endpoints. This is a decision in IntervalSets, which we can not change. The domain of integers is actually just 3:10. We tried to make things work for objects that are not <: Domain{T}. For example:

julia> d = uniondomain(1:3, 6:10)
1:3 ∪ 6:10

julia> 2 ∈ d
true

julia> 4 ∈ d
false

This example works cleanly, probably many others don't.

cscherrer commented 2 years ago

: is nice, except I don't know a way to use it to build infinite ranges. Maybe ℤ could be defined using Base.isinteger? Or using a range with Infinities.jl?

It's fine if these are removed, we'll just have to define the ones we need in MeasureBase. But I'm not sure of the best way to do that for ℤ.

daanhb commented 2 years ago

It could be in InfiniteArrays.jl? I believe the package defines ranges also

dlfivefifty commented 2 years ago

Yes InfiniteArrays.jl does not (yet) support bi-infinite ranges. This would be nice to have for bi-infinite operators (e.g. Fourier series) and pretty easy to add, just haven't had time. A PR would be appreciated.

Here is a quick mock-up (cf. OffsetArrays.IdOffsetRange). Note that you need to work around issues with AbstractUnitRange not expecting first(r) == -∞.

struct Integers{T<:Integer} <: AbstractUnitRange{T} end
Integers() = Integers{Int}()

@inline Base.axes(r::Integers) = (Base.axes1(r),)
@inline Base.axes1(r::Integers) = r
@inline Base.first(r::Integers) = -∞
@inline Base.last(r::Integers) = ∞
@inline Base.length(r::Integers) = ℵ₀
@inline Base.getindex(r::Integers{T}, k::Integer) where T = convert(T, k)::T
@inline Base.getindex(r::Integers{T}, k::AbstractUnitRange{<:Integer}) where T = convert(AbstractUnitRange{T}, k)
@inline Base.getindex(r::Integers{T}, k::StepRange{<:Integer}) where T = convert(StepRange{T}, k)
@inline Base.getindex(r::Integers{T}, k::AbstractVector{<:Integer}) where T = AbstractVector{T}(k)
dlfivefifty commented 2 years ago

One nice thing is this works:

julia> 5 in Integers()
true

julia> π in Integers()
false
daanhb commented 2 years ago

Should be in 0.5.5 now