JuliaApproximation / DomainSets.jl

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

improve 'hascanonicaldomain' function #97

Closed daanhb closed 2 years ago

daanhb commented 2 years ago

There was a possible StackOverflowError when comparing equality of domains using its generic fallback, because it called == again. In this PR, the fallback is kept for the time being, but the circular path is removed.

codecov[bot] commented 2 years ago

Codecov Report

Merging #97 (fa78848) into master (ddacdc6) will increase coverage by 0.32%. The diff coverage is 92.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   84.97%   85.29%   +0.32%     
==========================================
  Files          31       31              
  Lines        2422     2428       +6     
==========================================
+ Hits         2058     2071      +13     
+ Misses        364      357       -7     
Impacted Files Coverage Δ
src/generic/canonical.jl 72.72% <83.33%> (+2.95%) :arrow_up:
src/applications/coordinates.jl 100.00% <100.00%> (+100.00%) :arrow_up:
src/generic/lazy.jl 84.00% <100.00%> (+3.56%) :arrow_up:
src/generic/productdomain.jl 94.05% <100.00%> (ø)
src/maps/affine.jl 82.00% <0.00%> (-0.41%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ddacdc6...fa78848. Read the comment docs.

daanhb commented 2 years ago

Checking for equality of domains is a challenging problem, because it does not map to types. There are many domains equal to an interval, for example UnitBall{Float64}(). By implementing == generically for domains, we can perform some simplifications and test for equality of the underlying domains.

For example:

julia> UnitBall{Float64}() == ChebyshevInterval()
true

julia> ChebyshevInterval()^3 == ProductDomain(UnitBall{Float64}(), -1..1, ChebyshevInterval())
true

A domain can "opt in" to these kinds of transformations by implementing canonicaldomain. If it doesn't, then the fallback is canonicaldomain(d)=d. I now differentiate between these two situations using d === canonicaldomain(d), whereas it was d == canonicaldomain(d) before, which led to infinite recursion. Canonical domains, if not used, can now be safely ignored.

There is a lot of potential for ambiguity errors as well when comparing domains. When op is a binary operation, In several places I've used the pattern:

op(a, b) = op1(a,b)
op1(a,b) = op2(a,b)
op2(a,b) = default implementation

The idea here is that you can specialize op only if the combination of types is unique. If not, the rule is that op1 can specialize on the type of the first argument, and op2 on the type of the second argument. With this pattern, op1 can try to simplify a and invoke op(simplify(a),b), regardless of what b is and without causing ambiguity. op2 does the same for b. See here for an example.