move definition of Domain to DomainSetsCore package

[daanhb]

This is an up-to-date pull request for the possible move of Domain{T} to DomainSetsCore.jl, see #169

I did not move these two lines:

Base.IteratorSize(::Type{<:Domain}) = Base.SizeUnknown()
Base.isdisjoint(a::Domain, b::Domain) = isempty(a ∩ b)

as I'm not sure we should have these at the generic level of Domain in DomainSetsCore. For compatibility within this package, I kept them for intervals:

Base.IteratorSize(::Type{<:AbstractInterval}) = Base.SizeUnknown()
Base.isdisjoint(a::AbstractInterval, b::AbstractInterval) = isempty(a ∩ b)

[daanhb]

The build fails because DomainSetsCore hasn't been registered yet. All tests pass locally for me.

[hyrodium]

All changes in this PR look good to me.
domaineltype(::AbstractInterval) will be added in another PR, right? (#150 (comment))

[daanhb]

To be sure, in the current version the DomainSetsCore package does the following:

• it defines domaineltype for <:Domain{T} to be T
• it also defines eltype for <:Domain{T} to be T

The domaineltype function is new and "owned" by DomainSetsCore, so it can be implemented for any type that looks like a domain, without breaking anything (i.e. redefining eltype).
The definition of eltype for <:Domain{T} is added to avoid introducing any breaking changes. Currently IntervalSets defines eltype for intervals and DomainSets eltype defines it for any Domain, so removing it would be a breaking change. (I am quite sure that the eltype of intervals is used.)

[daanhb]

Hence, if IntervalSets would switch to using DomainSetsCore right now, the domaineltype of an interval will automatically be defined (because it is defined for Domain) and the eltype of an interval will also be defined and it will be the same as its domaineltype and the same as it is now. The definition of eltype for intervals in IntervalSets itself can be removed.

[daanhb]

Having reread a few issues (thanks for the link to gentype, interesting related discussion): moving the definition of Domain to a new package and settling the eltype/boundstype issue are two different things. In this PR, the latter issue is not solved, only postponed, it's best to be clear on that. I'll make an attempt later to summarize different long-term possible outcomes. I am not in a rush to go and register anything so things can still change.

[hyrodium]

Thank you for the quick detailed response!

I totally agree that IntervalSets.jl should not own Domain{T}, but the interpretation of the parameter T is closely related to the eltype-issue.

Proposal

Reconsidered a little, I think we have the following choices A, B, and C.

A

• IntervalSets.jl depends on DomainSetsCore.jl.
• Domain does not have the type parameter T.
• domaineltype(1..2) will be Float64.
• eltype(::Type{AbstractInterval{T}}) is not defined.

# Defined in IntervalSets.jl
abstract type AbstractInterval{T} <: DomainSetsCore.Domain end
DomainSetsCore.domaineltype(::Interval{L,R,T}) where {L,R,T} = float(T)

Domain does not have domain eltype as a type parameter. domaineltype should be defined for each type.

B

• IntervalSets.jl does not depend on DomainSetsCore.jl.
• supertype(AbstractInterval) will be Any
• domaineltype(1..2) will be Float64.
• eltype(::Type{AbstractInterval{T}}) is not defined.

# Defined in IntervalSets.jl
abstract type AbstractInterval{T} end

# Defined in some package extension
DomainSetsCore.domaineltype(::IntervalSets.Interval{L,R,T}) where {L,R,T} = float(T)
DomainSetsCore.DomainStyle(::IntervalSets.AbstractInterval) = IsDomain()

IntervalSets.jl do not care the eltype-issue. DomainSetsCore.domaineltype will handle it.

C

• IntervalSets.jl depends on DomainSetsCore.jl.
• Domain has the type parameter T.
• domaineltype(1..2) will be Int.
• eltype(::Type{AbstractInterval{T}}) is not defined.

# Defined in IntervalSets.jl
abstract type AbstractInterval{T} <: DomainSetsCore.Domain{T} end

The function domaineltype returns "typical eltype" which may be the type of the boundaries of an interval.

Conclusion

I am not particularly sure which option, A, B, or C, is the best.
Note that the following properties should be satisfied in any case.

• eltype(::Type{AbstractInterval{T}}) is not defined (unless Random.gentype will be removed.).
• T in AbstractInterval{T} represents the type of the boundaries.

Sorry if I missed anything in our past discussions so far.

[daanhb]

Thanks for the overview, that helps to clarify the issues. I agree that these are possibilities, with pros and cons to each, but for the sake of completeness I think there are at least two more:

D

Intervals have two type parameters: AbstractInterval{B,T} <: Domain{T} in which B is the boundstype and T is the domaineltype. This allows a user to have precise integer endpoints such as 1..2, while having control over the type of elements generated by code using intervals.

E

We don't change anything and just move Domain{T} to the core package. We document what the meaning is of boundstype, eltype and/or domaineltype, and live with the possibility that every once in a while someone discovers the weird eltype of 1..2 and is surprised.

F

Like B, but AbstractInterval does not have a T parameter at all.

Some further observations and perspectives

• From the point of view of domains, if there is a T in Domain{T} then it can not be a boundstype because the notion of boundstype does not generalize. In general, the boundary of a domain is again a domain (e.g., the boundary of a disk is a circle) and hence it is associated with a set and not with a type. The boundstype is specific to intervals, which has a finite number of boundary points that can actually be stored, and even more specifically it refers to the type that was chosen in that representation. (An analogy would be a disk with a center, and having a centertype to describe the type of the center point. It is valid, but not generic.)

• A general notion of boundstype could be that it is the type of points on the boundary, but in that case it is by definition also a valid element type for the set. This is true also for intervals, as a closed interval actually includes the endpoints, and it is why using T both as the element type and as the type to represent the endpoints actually works in many cases. (A disk with a center could also have a centertype equal to the element type of the set.)

• From the mathematical perspective a closed interval is the set of all elements x such that a <= x <= b. The domaineltype is an arbitrary restriction of the type of x, and the boundstype is an arbitrary restriction of the types of a and b. It is only in Julia that one starts caring about the types.

• If you store a and b then you can't do without a boundstype, but if you don't store a and b you can (i.e. a fixed interval). You can always do without an element type. The latter is what makes the two-parameter solution D look like overkill and the former is why I added F.

[daanhb]

Looking at options:

• about A: to me there is no genuine difference between Domain without T and all domains defining domaineltype, and having Domain{T}. It makes an implementation difference in that type parameters may be saved (i.e. it avoids option D) but it makes no functional difference.
• about B: it is interesting that it is even possible. I still fear problems down the road. In particular if an interval is not a domain, then its behaviour should not change when other packages involving domains are loaded. Even a useful syntax like 0..1 × 2..3 to create product domains would be forbidden, the alternative being productdomain(0..1, 2..3). (Very strictly speaking 0..1 × 2..3 is an error in IntervalSets and defining it to be a product domain changes behaviour regardless of the inheritance involved. By using DomainSetsCore, a package somehow declares to be okay with that. It's certainly worse if DomainSetsCore is not a dependency.)

Currently, option C seems most viable to me. We can remove the definition of eltype from DomainSetsCore and stick to domaineltype, continue to define eltype in IntervalSets and DomainSets as it is now, and then deprecate those definitions. Since the T in AbstractInterval{T} plays the role of both the boundstype and the domaineltype, anyone who is interested in a particular element type can use IntervalSets (by typing 1..2.0 instead of 1..2), and anyone who is not interested simply doesn't care. Perhaps domaineltype(1..2) == Int is less surprising than eltype(1..2) == Int: at least it is clear where to look for the documentation.

[daanhb]

To add, I'm not in favour of any solution in which AbstractInterval{T} <: Domain{T} but domaineltype differs from T. That is not viable.

[daanhb]

For the sake of the argument, I disagree with the idea that there is no iteration involved in the definition of intervals (and hence no link with eltype). The name IntervalSets itself refers to a set. It is impossible to iterate over all elements in practice of course, but there is nothing wrong with the concept of saying "for each x in..."

For example:

julia> using DomainIntegrals, IntervalSets

julia> integral(exp(x) for x in 0..1)
1.7182818284590453

julia> integral(exp(x) for x in 0..big(1))
1.718281828459045235360287471352662497757247093699959574966967627724076630353935

This notation defines the function exp(x) on the interval [0,1] by defining its value for each x in that interval. The definition of the interval hints at the expected precision type of the approximation.

[dlfivefifty]

I thought the plan was to move to more of an interface than an abstract type? Why not use this as an opportunity to just delete Domain{T} once and for all?

The interface could be domaineltype and in.

[daanhb]

I thought the plan was to move to more of an interface than an abstract type? Why not use this as an opportunity to just delete Domain{T} once and for all?

Let's not discuss that here. That is a design question for the new core package.

[daanhb]

Okay to register DomainSetsCore.jl? Since the discussion here I've removed eltype from the package, so we are in scenario C. Using DomainSetsCore as it is now is a non-breaking change for IntervalSets and DomainSets and could happen anytime. Removing eltype later on is technically breaking.