JuliaApproximation / DomainSets.jl

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

Implementing domains as an interface #141

Closed daanhb closed 9 months ago

daanhb commented 10 months ago

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 ( https://github.com/JuliaMath/IntervalSets.jl/issues/115, https://github.com/JuliaMath/IntervalSets.jl/issues/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 commented 10 months ago

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 commented 10 months ago

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[bot] commented 10 months ago

Codecov Report

Patch coverage: 79.90% and project coverage change: -4.81% :warning:

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

Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation) | Coverage Δ | | |---|---|---| | [src/DomainSets.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL0RvbWFpblNldHMuamw=) | `100.00% <ø> (ø)` | | | [src/domains/cube.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2RvbWFpbnMvY3ViZS5qbA==) | `96.50% <ø> (ø)` | | | [src/domains/interval.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2RvbWFpbnMvaW50ZXJ2YWwuamw=) | `57.32% <41.66%> (-31.50%)` | :arrow_down: | | [src/generic/core.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2dlbmVyaWMvY29yZS5qbA==) | `58.33% <58.33%> (ø)` | | | [src/domains/trivial.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2RvbWFpbnMvdHJpdmlhbC5qbA==) | `90.54% <62.50%> (-3.83%)` | :arrow_down: | | [src/domains/indicator.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2RvbWFpbnMvaW5kaWNhdG9yLmps) | `86.20% <66.66%> (+3.44%)` | :arrow_up: | | [src/generic/canonical.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2dlbmVyaWMvY2Fub25pY2FsLmps) | `71.42% <66.66%> (-1.91%)` | :arrow_down: | | [src/generic/domain.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2dlbmVyaWMvZG9tYWluLmps) | `86.20% <80.00%> (-10.23%)` | :arrow_down: | | [src/util/common.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL3V0aWwvY29tbW9uLmps) | `93.40% <80.00%> (-2.05%)` | :arrow_down: | | [src/generic/setoperations.jl](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation#diff-c3JjL2dlbmVyaWMvc2V0b3BlcmF0aW9ucy5qbA==) | `88.05% <83.33%> (-4.17%)` | :arrow_down: | | ... and [11 more](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation) | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/JuliaApproximation/DomainSets.jl/pull/141/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaApproximation)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dlfivefifty commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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).

ChrisRackauckas commented 5 months ago

What was breaking in this release?

daanhb commented 5 months ago

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).