Fix Stackoverflow when creating interval of intervals #185

Kolaru · 2018-07-20T22:54:47Z

Fix #182. I have define a new type alias of all supported type (basically all built-in number type). Therefore this make the implementation the less generic possible, which is also the safest.

The change also somehow confuse Julia when it comes to finding the most specific method in some cases, for unknown reason.

Finally a small change in the display function to be sure to avoid an infinite recursion by using specifically the BigFloat constructor.

coveralls · 2018-07-20T23:11:49Z

Coverage remained the same at 92.444% when pulling 43a1073 on Kolaru:fix_display_recursion into cab2433 on JuliaIntervals:master.

coveralls · 2018-07-20T23:11:50Z

Coverage decreased (-0.07%) to 90.958% when pulling 337ce67 on Kolaru:fix_display_recursion into 4d0eba5 on JuliaIntervals:master.

codecov-io · 2018-07-20T23:23:19Z

Codecov Report

❗ No coverage uploaded for pull request base (master@1516c5e). Click here to learn what that means.
The diff coverage is 88.88%.

@@            Coverage Diff            @@
##             master     #185   +/-   ##
=========================================
  Coverage          ?   82.31%           
=========================================
  Files             ?       25           
  Lines             ?     1227           
  Branches          ?        0           
=========================================
  Hits              ?     1010           
  Misses            ?      217           
  Partials          ?        0

Impacted Files	Coverage Δ
src/display.jl	`80.61% <ø> (ø)`
src/intervals/arithmetic.jl	`94.11% <ø> (ø)`
src/intervals/intervals.jl	`87.23% <88.88%> (ø)`

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 1516c5e...b4fbd97. Read the comment docs.

lbenet · 2018-09-27T16:28:03Z

Thanks @Kolaru for looking into this. I just checked in Julia 1.0 and the problem reported in #182 persists.

I like your idea of introducing IASupportedType here. However, your approach may be too restrictive, in the sense that a user may introduce another struct which is subtype of Real, for which it makes sense to define intervals; an example could be DoubleDouble or ArbFloat.

I think the following would allow for such an enlargement, but I haven't being able to test it in Julia 1.0.

using InteractiveUtils: subtypes
const IASupportedType = Union{setdiff(subtypes(Real), [AbstractInterval])...}

This would result on IASupportedType being Union{AbstractIrrational, AbstractFloat, Integer, Rational}, which is essentially what you listed.

Another option, as @dpsanders suggested, is that Interval(::Interval) throws an error.

Kolaru · 2018-09-28T15:51:10Z

What about a trait-like solution ? Something like

struct Interval{T} where {T <: Real}
  a::T
  b::T
  Interval{T}(::Type{Val{true}}, a::T, b::T) = new(a, b)
  Interval{T}(::Type{Val{false}}, ::T, ::T) = error("Type $T not good for interval arithmetic.")
end

Interval(a::T, b::T) where {T <: Real} = Interval(Val{isGoodForIA(T)}, a, b)

So to make a custom type A usable in interval, the user would simply have to write

isGoodForIA(A) = true

Like that absurd intervals are never passed silently, but it can still easily be extended with arbitrary type. Also I don't think this would justifiy adding SimpleTraits.jl as dependency, since it is quite simple still.

dpsanders

Sorry for the far too long delay in getting to this. I think this is very nice.

I've added some suggestions for simplifying the code.
Basically, the only place where type restrictions should be required is when actually creating an Interval object.

dpsanders · 2019-03-04T22:17:16Z

src/intervals/arithmetic.jl

@@ -447,7 +447,7 @@ function radius(a::Interval)
    return max(m - a.lo, a.hi - m)
 end

-function radius(a::Interval{Rational{T}}) where T
+function radius(a::Interval{Rational{T}}) where {T <: Integer}


I don't think it's necessary to specify T <: Integer since that is imposed when a Rational is created.

dpsanders · 2019-03-04T22:17:48Z

src/intervals/arithmetic.jl

@@ -461,7 +461,7 @@ Return the unique interval `c` such that `b+c=a`.
 See Section 12.12.5 of the IEEE-1788 Standard for
 Interval Arithmetic.
 """
-function cancelminus(a::Interval{T}, b::Interval{T}) where T<:Real
+function cancelminus(a::Interval{T}, b::Interval{T}) where {T <: IASupportedType}


I think we can just leave this as where {T}. I don't see the need to restrict the type here; the type should be restricted already when an Interval is initially created.

dpsanders · 2019-03-04T22:18:37Z

src/intervals/intervals.jl

@@ -11,52 +11,56 @@ else
    const validity_check = false
 end

+const IASupportedType = Union{Integer, Rational, Irrational, Float16, Float32,


I don't think it's a good idea to make a restrictive list like this. I like the trait idea.

I kept it in the latest update, for the definition of the default cases for can_bound_interval.

dpsanders · 2019-03-04T23:05:05Z

src/intervals/intervals.jl

 abstract type AbstractInterval{T} <: Real end

 struct Interval{T<:Real} <: AbstractInterval{T}
    lo :: T
    hi :: T

-    function Interval{T}(a::Real, b::Real) where T<:Real
-
+    function Interval{T}(::Type{Val{true}}, a::Real, b::Real) where {T <: Real}


Change to ::Val{true}

dpsanders · 2019-03-04T23:05:48Z

src/intervals/intervals.jl

-
+## Traits functions
+canBeIntervalBound(::Type{T}) where {T <: IASupportedType} = true
+canBeIntervalBound(T) = false  # Default case


Maybe canBeIntervalBound(::Type{T}) where {T} = false

dpsanders · 2019-03-04T23:06:55Z

src/intervals/intervals.jl


-Interval(a::T, b::T) where T<:Real = Interval{T}(a, b)
-Interval(a::T) where T<:Real = Interval(a, a)
+Interval{T}(a::Real, b::Real) where {T <: Real} = Interval{T}(Val{canBeIntervalBound(T)}, a, b)


I guess you can now remove the <: Real

Replace by Val(can_bound_interval(T))

dpsanders · 2019-03-04T23:07:15Z

src/intervals/intervals.jl

    end
 end

-
+## Traits functions
+canBeIntervalBound(::Type{T}) where {T <: IASupportedType} = true


rename to can_bound_interval

dpsanders · 2019-03-04T23:07:37Z

src/intervals/intervals.jl

 Interval(a::Tuple) = Interval(a...)
-Interval(a::T, b::S) where {T<:Real, S<:Real} = Interval(promote(a,b)...)
+Interval(a::T, b::S) where {T <: Real, S <: Real} = Interval(promote(a,b)...)


Remove <: Real

dpsanders · 2019-03-04T23:07:58Z

src/intervals/intervals.jl


 ## Concrete constructors for Interval, to effectively deal only with Float64,
 # BigFloat or Rational{Integer} intervals.
-Interval(a::T, b::T) where T<:Integer = Interval(float(a), float(b))
-Interval(a::T, b::T) where T<:Irrational = Interval(float(a), float(b))
+Interval(a::T, b::T) where {T <: Integer} = Interval(float(a), float(b))


I think these <: can now be removed.

dpsanders · 2019-03-04T23:08:18Z

src/intervals/intervals.jl


-eltype(x::Interval{T}) where T<:Real = T
+eltype(x::Interval{T}) where {T <: Real} = T


Remove <: Real

Kolaru · 2019-03-27T22:16:59Z

I closed the wrong PR, sorry for this.

dpsanders · 2019-06-24T15:59:02Z

What's the status here? Should we merge this?

Kolaru · 2019-06-28T00:01:34Z

I need to go back to it to address your last review. I don't remember why I didn't do it earlier, but I'll do it shortly.

Kolaru · 2019-08-16T01:47:37Z

The coverage decrease comes from the fact that can_bound_interval is not found to be hit by Coveralls, probably because the method is always optimized out by the compiler. I don't think that makes much sense to test it directly, so this PR should be ready.

Kolaru · 2020-10-26T00:08:54Z

This is ready (and may also help avoid some amiguity error I get in #271 when trying to have generic constructors).

codecov-commenter · 2021-06-12T01:22:23Z

Codecov Report

Merging #185 (09a4036) into master (a3dc27f) will decrease coverage by 0.02%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
- Coverage   91.55%   91.52%   -0.03%     
==========================================
  Files          25       25              
  Lines        1752     1758       +6     
==========================================
+ Hits         1604     1609       +5     
- Misses        148      149       +1

Impacted Files	Coverage Δ
src/intervals/arithmetic.jl	`97.20% <50.00%> (ø)`
src/intervals/intervals.jl	`91.42% <92.30%> (-0.76%)`	⬇️
src/IntervalArithmetic.jl	`100.00% <100.00%> (ø)`
src/display.jl	`88.81% <100.00%> (ø)`

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 a3dc27f...09a4036. Read the comment docs.

Kolaru force-pushed the fix_display_recursion branch from 43a1073 to 09aa126 Compare September 29, 2018 01:33

This was referenced Dec 6, 2018

X ∈ X is true when X is an interval #247

Closed

Add "in" for IntervalBox #244

Merged

dpsanders reviewed Mar 4, 2019

View reviewed changes

Kolaru closed this Mar 27, 2019

Kolaru reopened this Mar 27, 2019

Kolaru mentioned this pull request Jul 3, 2019

Remove Interval <: Real ? #237

Closed

Kolaru mentioned this pull request Aug 15, 2019

Fix convert bug #320

Closed

Kolaru force-pushed the fix_display_recursion branch from 09aa126 to b4fbd97 Compare August 16, 2019 00:30

Kolaru added 6 commits October 25, 2020 23:18

Initial fix

6aa2541

Add tests

882d921

Implement trait-like supported type for interval bounds

8e7a7fc

Address review

1882a98

Fix cancelminus stack overflow

283f878

Fix misc from rebase

337ce67

Kolaru force-pushed the fix_display_recursion branch from b4fbd97 to 337ce67 Compare October 26, 2020 00:02

Merge branch 'master' into fix_display_recursion

09a4036

OlivierHnt closed this Jul 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Stackoverflow when creating interval of intervals #185

Fix Stackoverflow when creating interval of intervals #185

Kolaru commented Jul 20, 2018

coveralls commented Jul 20, 2018

coveralls commented Jul 20, 2018 •

edited

Loading

codecov-io commented Jul 20, 2018 •

edited

Loading

lbenet commented Sep 27, 2018

Kolaru commented Sep 28, 2018 •

edited

Loading

dpsanders left a comment

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

Kolaru Aug 16, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

dpsanders Mar 5, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

dpsanders Mar 4, 2019

Kolaru commented Mar 27, 2019 •

edited

Loading

dpsanders commented Jun 24, 2019

Kolaru commented Jun 28, 2019

Kolaru commented Aug 16, 2019 •

edited

Loading

Kolaru commented Oct 26, 2020

codecov-commenter commented Jun 12, 2021 •

edited

Loading


		eltype(x::Interval{T}) where T<:Real = T
		eltype(x::Interval{T}) where {T <: Real} = T

Fix Stackoverflow when creating interval of intervals #185

Fix Stackoverflow when creating interval of intervals #185

Conversation

Kolaru commented Jul 20, 2018

coveralls commented Jul 20, 2018

coveralls commented Jul 20, 2018 • edited Loading

codecov-io commented Jul 20, 2018 • edited Loading

Codecov Report

lbenet commented Sep 27, 2018

Kolaru commented Sep 28, 2018 • edited Loading

dpsanders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kolaru commented Mar 27, 2019 • edited Loading

dpsanders commented Jun 24, 2019

Kolaru commented Jun 28, 2019

Kolaru commented Aug 16, 2019 • edited Loading

Kolaru commented Oct 26, 2020

codecov-commenter commented Jun 12, 2021 • edited Loading

Codecov Report

coveralls commented Jul 20, 2018 •

edited

Loading

codecov-io commented Jul 20, 2018 •

edited

Loading

Kolaru commented Sep 28, 2018 •

edited

Loading

Kolaru commented Mar 27, 2019 •

edited

Loading

Kolaru commented Aug 16, 2019 •

edited

Loading

codecov-commenter commented Jun 12, 2021 •

edited

Loading