JuliaApproximation / DomainSets.jl

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

Fix ℝ == ℝ #96

Closed dlfivefifty closed 2 years ago

dlfivefifty commented 2 years ago

Fixes the following:

julia> ℝ == ℝ
ERROR: StackOverflowError:
Stacktrace:
     [1] simplifies(d::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:54
     [2] isequal1(d1::RealNumbers, d2::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:120
     [3] ==(d1::RealNumbers, d2::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:118
     [4] isdifferentfrom(d1::RealNumbers, d2::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:19
     [5] hascanonicaldomain(ctype::DomainSets.Equal, d::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:40
--- the last 5 lines are repeated 15995 more times ---
 [79981] simplifies(d::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:54
 [79982] isequal1(d1::RealNumbers, d2::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:120
 [79983] ==(d1::RealNumbers, d2::RealNumbers)
       @ DomainSets ~/Projects/DomainSets.jl/src/generic/canonical.jl:118

Probably a better fix is needed, e.g., by implementing canonicaldomain, but I don't quite understand what should be done.

codecov[bot] commented 2 years ago

Codecov Report

Merging #96 (e52b18e) into master (6498443) will increase coverage by 0.03%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   84.94%   84.97%   +0.03%     
==========================================
  Files          31       31              
  Lines        2417     2422       +5     
==========================================
+ Hits         2053     2058       +5     
  Misses        364      364              
Impacted Files Coverage Δ
src/domains/numbers.jl 95.23% <100.00%> (+1.48%) :arrow_up:

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 6498443...e52b18e. Read the comment docs.

daanhb commented 2 years ago

The underlying problem lies here. I wrote a generic fallback for equality of domains, in terms of "simplifications" of domains, but that wasn't the best idea because it seems to replace the standard (implicitly defined) equality of all concrete domains. I only realized that later. That's why you had to implement equality of these number sets explicitly, the fix is correct. The better fix would be to remove that generic equality. I have to think about it, canonical domains should be opt-in only and not interfere with anything otherwise.

daanhb commented 2 years ago

See #97