Implementing domains as an interface #141

daanhb · 2023-08-28T13:51:17Z

I think this is a candidate for becoming 0.7.

It mostly implements the idea of a domain as an interface (#14, #120) and addresses some related issues ( JuliaMath/IntervalSets.jl#115, JuliaMath/IntervalSets.jl#136). The main goal is to improve interoperability with other packages.

Among other things this PR includes a copy of the contents of a newly proposed DomainSetsCore.jl package, with the exception of the definition of Domain{T} which remains in IntervalSets.jl for now. See core.jl for the code and the README of DomainSetsCore for a formal definition of the "domain interface". If this works well, we can still register DomainSetsCore and move the definitions there.

Most importantly this PR implements all operations of DomainSets for any type, whether it inherits from Domain or not, except for functions that have a standard name outside of DomainSets. In that case, those functions are only implemented for variables of type Domain or variables which are passed "as a domain" (using the AsDomain(d)) syntax. A prominent example is eltype. An alternative function which is "owned" by DomainSets is domaineltype.

As an example of interoperability, see here for a package extension to make domains co-exist with types of GeometryBasics.jl. No changes to either packages are required, hence it could be an extension in either of the two packages. I've illustrated here what it takes for intervals of IntervalSets.jl and Intervals.jl to interact with each other.

An example of the usefulness is that in a package like DomainIntegrals.jl one can specify the integral domain as any domain (an interval from IntervalSets.jl, or from Intervals.jl, or a rectangle from GeometryBasics.jl) and it "just works". (Up to possible bugs and missing functionality for now, of course....)

daanhb · 2023-08-28T13:52:38Z

I think these changes are almost completely backwards compatible with the 0.6 version of DomainSets, because functionality has mostly been added. However, the internal change is quite substantial, and I'm pretty sure that I've changed the default behaviour of == and issubset for domains, so a version upgrade is called for.

daanhb · 2023-08-28T13:56:12Z

Also, a note on eltype. Both IntervalSets.jl and Intervals.jl implement eltype because they see intervals as sets. However, both can have an Int eltype when the interval has integer endpoints. This creates funny situations in computations. The domains in GeometryBasics.jl and in Meshes.jl do not have an eltype (their eltype is Any), but they all implicitly work with vectors of fixed length DIM and element type T. Finally, the sets in LazySets.jl have an eltype of Float64 even when they describe sets of vectors of floats.

The only solution is to create a new function, which I've called domaineltype. It has a sensible definition for all domains in the packages mentioned above. For now, internally this PR still uses eltype for variables of type Domain{T} and that eltype is T. It uses domaineltype (abbreviated to deltype internally) for variables which represent domains but whose type may be something else.

codecov · 2023-08-28T13:57:53Z

Codecov Report

Patch coverage: 79.90% and project coverage change: -4.81% ⚠️

Comparison is base (f9638be) 85.70% compared to head (1162e66) 80.90%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
- Coverage   85.70%   80.90%   -4.81%     
==========================================
  Files          34       35       +1     
  Lines        2659     2697      +38     
==========================================
- Hits         2279     2182      -97     
- Misses        380      515     +135

Files Changed	Coverage Δ
src/DomainSets.jl	`100.00% <ø> (ø)`
src/domains/cube.jl	`96.50% <ø> (ø)`
src/domains/interval.jl	`57.32% <41.66%> (-31.50%)`	⬇️
src/generic/core.jl	`58.33% <58.33%> (ø)`
src/domains/trivial.jl	`90.54% <62.50%> (-3.83%)`	⬇️
src/domains/indicator.jl	`86.20% <66.66%> (+3.44%)`	⬆️
src/generic/canonical.jl	`71.42% <66.66%> (-1.91%)`	⬇️
src/generic/domain.jl	`86.20% <80.00%> (-10.23%)`	⬇️
src/util/common.jl	`93.40% <80.00%> (-2.05%)`	⬇️
src/generic/setoperations.jl	`88.05% <83.33%> (-4.17%)`	⬇️
... and 11 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dlfivefifty · 2023-08-29T05:30:35Z

Probably the behaviour that makes most logical sense is eltype(0..1) == Real. But then there could be boundaryeltype(0..1) == Int. (Probably needs a more general name…)

then wherever is calling eltype now could call float(boundaryeltype(d))

daanhb · 2023-08-30T08:04:02Z

In the current design I'd say that float(boundaryeltype(d)) is a good candidate implementation for eltype or domaineltype of intervals with numeric types.

Yes, eltype being Real makes conceptual sense, but it does not convey any useful information. For example, if rand for an interval returns a point in the interval, which will be the type of that point? That is what T is for <:Domain{T} and what domaineltype is for other objects satisfying the domain interface.

daanhb · 2023-09-05T12:06:34Z

Okay to merge this?
It only adds behaviour, which we can test in more cases before making other changes (like moving the definition of Domain).

dlfivefifty · 2023-09-05T12:16:55Z

src/domains/boundingbox.jl

 unionbox1(d1, d2) = unionbox2(d1, d2)
 unionbox2(d1, d2) = fullspace(d1)
 unionbox1(d1::EmptySpace, d2) = d2
 unionbox1(d1::FullSpace, d2) = d1
 unionbox2(d1, d2::EmptySpace) = d1
 unionbox2(d1, d2::FullSpace) = d2

-unionbox(d1::D, d2::D) where {D<:FixedInterval} = d1
+unionbox1(d1::D, d2::D) where {D<:FixedInterval} = d1


What's unionbox1 and below intersectbox1?

unionbox(d1,d2) is a function of two arguments, which sometimes simplifies for certain d1 (independently of d2 or for some possibilities of d2). Similarly, it may simplify for certain d2. The call chain goes:
unionbox(d1,d2) -> unionbox1(d1,d2) -> unionbox2(d1,d2) -> some_default_algorithm

The idea is that unionbox1 can dispatch safely (without ambiguity) solely on d1, and unionbox2 can dispatch on d2. I've used this pattern in many places and it works rather well. If a combination of d1 and d2 is specific enough, then one can dispatch on it using just the unionbox function.

dlfivefifty · 2023-09-05T12:19:09Z

src/domains/interval.jl

@@ -244,8 +248,8 @@ function similar_interval(d::HalfLine{T,C}, a::S, b::S) where {T,S,C}
    HalfLine{promote_type(float(T),S),C}()
 end

-point_in_domain(d::NonnegativeRealLine) = zero(eltype(d))
-point_in_domain(d::PositiveRealLine) = one(eltype(d))
+point_in_domain(d::NonnegativeRealLine) = zero(deltype(d))


Probably this name is clearer, but this is called a "choice function" in the Axiom of Choice so we could just call this choice(d)

Hmm, it would be good if choice(s) was defined for a Set too, for example, but it isn't. This function really is just for testing purposes, but it could have any name.

Just add choice(d::AbstractSet) = first(d)?

iterate(d)[1] is kind of the same...

the point_in_domain function roughly speaking picks some point in the interior of a domain, if possible. It may be good to think about how to "sample" points from a domain more generally (like rand), but that is a separate issue.

Okay, I'm renaming to choice and adding a deprecation for point_in_domain

src/generic/domain.jl

ChrisRackauckas · 2024-02-15T11:13:07Z

What was breaking in this release?

daanhb · 2024-02-16T13:26:56Z

Syntactically, normally nothing. A lot of the internals changed though, which mainly results in more functionality (as opposed to functionality changing) for types that do not inherit from the main Domain type.

There were some nuances in the decision whether two domains are equal which may have changed and which prompted a new series, out of caution. I would expect 0.7 to work as is for most people who had been using 0.6.x (and I'm assuming you're asking because you didn't notice a difference).

daanhb added 10 commits August 22, 2023 09:16

tests pass

965dbe6

remove AsDomain for intervals

beefcf1

fixes

ce52f2e

use deltype instead of eltype whenever it seems more general

0340a44

use domaineltype in more places

c1b143a

make tests pass again

7e7a81b

numtype and prectype

29262a1

package extension and improving interface

7cb7d94

try some package extensions

6f60042

include defs of domainsetscore

3ee18e1

daanhb mentioned this pull request Aug 28, 2023

Move definition of Domain to DomainSetsCore.jl package #140

Closed

dlfivefifty reviewed Sep 5, 2023

View reviewed changes

daanhb added 3 commits September 5, 2023 14:33

avoid deltype and other abbreviations

a7cef5a

introduce choice function

272c4ea

avoid unicode hyphen error

1162e66

daanhb mentioned this pull request Sep 7, 2023

Deprecate methods for eltype and add boundstype JuliaMath/IntervalSets.jl#150

Open

daanhb mentioned this pull request Oct 9, 2023

Should boundary(::Rectangle) use simpler types? #142

Closed

daanhb merged commit 4daf8a3 into master Oct 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing domains as an interface #141

Implementing domains as an interface #141

daanhb commented Aug 28, 2023

daanhb commented Aug 28, 2023

daanhb commented Aug 28, 2023

codecov bot commented Aug 28, 2023 •

edited

Loading

dlfivefifty commented Aug 29, 2023

daanhb commented Aug 30, 2023

daanhb commented Sep 5, 2023

dlfivefifty Sep 5, 2023

daanhb Sep 5, 2023 •

edited

Loading

dlfivefifty Sep 5, 2023

daanhb Sep 5, 2023

dlfivefifty Sep 5, 2023

dlfivefifty Sep 5, 2023

daanhb Sep 5, 2023

daanhb Sep 5, 2023

ChrisRackauckas commented Feb 15, 2024

daanhb commented Feb 16, 2024

Implementing domains as an interface #141

Implementing domains as an interface #141

Conversation

daanhb commented Aug 28, 2023

daanhb commented Aug 28, 2023

daanhb commented Aug 28, 2023

codecov bot commented Aug 28, 2023 • edited Loading

Codecov Report

dlfivefifty commented Aug 29, 2023

daanhb commented Aug 30, 2023

daanhb commented Sep 5, 2023

dlfivefifty Sep 5, 2023

Choose a reason for hiding this comment

daanhb Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

dlfivefifty Sep 5, 2023

Choose a reason for hiding this comment

daanhb Sep 5, 2023

Choose a reason for hiding this comment

dlfivefifty Sep 5, 2023

Choose a reason for hiding this comment

dlfivefifty Sep 5, 2023

Choose a reason for hiding this comment

daanhb Sep 5, 2023

Choose a reason for hiding this comment

daanhb Sep 5, 2023

Choose a reason for hiding this comment

ChrisRackauckas commented Feb 15, 2024

daanhb commented Feb 16, 2024

codecov bot commented Aug 28, 2023 •

edited

Loading

daanhb Sep 5, 2023 •

edited

Loading