move definition of Domain to DomainSetsCore package
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)
The build fails because DomainSetsCore hasn't been registered yet. All tests pass locally for me.
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.)
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.
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.
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.
Domaindoes not have the type parameterT.domaineltype(1..2)will beFloat64.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 beAnydomaineltype(1..2)will beFloat64.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.
Domainhas the type parameterT.domaineltype(1..2)will beInt.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 (unlessRandom.gentypewill be removed.).TinAbstractInterval{T}represents the type of the boundaries.
Sorry if I missed anything in our past discussions so far.
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
TinDomain{T}then it can not be aboundstypebecause the notion ofboundstypedoes 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. Theboundstypeis 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 acentertypeto describe the type of the center point. It is valid, but not generic.) -
A general notion of
boundstypecould 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 usingTboth 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 acentertypeequal to the element type of the set.) -
From the mathematical perspective a closed interval is the set of all elements
xsuch thata <= x <= b. Thedomaineltypeis an arbitrary restriction of the type ofx, and theboundstypeis an arbitrary restriction of the types ofaandb. It is only in Julia that one starts caring about the types. -
If you store
aandbthen you can't do without aboundstype, but if you don't storeaandbyou 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.
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..3to create product domains would be forbidden, the alternative beingproductdomain(0..1, 2..3). (Very strictly speaking0..1 × 2..3is 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.
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.
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.
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.
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.
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.
FYI, we're preparing a breaking update of DomainSets. With some delay... :-) It might be an idea to proceed? This package has several open issues and pull requests related to DomainSets, I'd like to clear it up.
It could go like this:
- DomainSetsCore is registered. It defines the Domain{T} type and defines domaineltype(::Domain{T}) to be T, but IntervalSets is oblivious to that
- IntervalSets updates to use DomainSetsCore instead of defining Domain{T} itself. This is a non-breaking change that could happen any time (right?)
- IntervalSets introduces boundstype and deprecates eltype. The latter would be breaking.
Meanwhile, I'm exploring how to change DomainSets such that no type piracy is left. Depending on how that goes it can go into v0.8, or into a later v0.9.
I think everyone involved was mostly fine with that, perhaps up to a name change to ContinuousSetsCore.jl or something like that. The main alternative is for IntervalSets to just remove the Domain{T} type. That is mainly a breaking change for DomainSets, not here, for which I do expect some fallback but would still rather do that before v1 of either package.