-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement interval types that are not Real #254
Conversation
I finally had a look at the code, and I like the approach of using three-valued logic for this problem. Maybe we could make the names more expressive; for example by calling the standard-compliant (i.e. set-like) type I'd like to point out that the so called (As a side note, the IEEE-compliant type (I guess currently Anyway, these are technical details that can probably be addressed easily in the |
|
||
disambiguation_mode(val) = NoDisambiguation | ||
disambiguation_mode(::Union{SetInterval, RealSetInterval}) = ErrorOnAmbiguous | ||
disambiguation_mode(::PseudoNumberInterval) = FalseOnAmbiguous |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the intervals should behave more like:
disambiguation_mode(::PseudoNumberInterval) = FalseOnAmbiguous | |
disambiguation_mode(::Union{SetInterval, RealSetInterval}) = IEEEConform | |
disambiguation_mode(::PseudoNumberInterval) = ErrorOnAmbiguous |
(Here IEEEConform
is the current behavior.) It's not quite clear to me if FalseOnAmbiguous
would add anything since one could always wrap an ErrorOnAmbiguous
type in a try-catch
statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea behind FalseOnAmbiguous
was that it return true
if and only if the tested predicate is guaranteed to be true, otherwise returning false
. It turns out that for comparison, the IEEE standard (section 10.5.10) call the corresponding function precedes
(and denotes it with a funky operator), while the less
operator (denoted as <
in the standard) represents something else.
Your are correct that this should simply refers to the standard and in the documentation it must be clear what we call <
. It will be hard to be fully compliant with the notation of the standard in a meaningful way, because I don't know if it takes in account languages supporting unicode operators, nor the very Julian expectation of being able to throw new types into old algorithms.
Then I totally messed up the naming, the standard calling the described default interval as "set based", using the same convention we end up to your proposition (that I do not commit yet as IEEEConform
is not defined).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your are correct that this should simply refers to the standard and in the documentation it must be clear what we call
<
. It will be hard to be fully compliant with the notation of the standard in a meaningful way, because I don't know if it takes in account languages supporting unicode operators, nor the very Julian expectation of being able to throw new types into old algorithms.
I don't think that is an issue considering IEEE section 12.2 "Naming conventions for operations" states that implementations can use 'generic text names' like add(a, b)
instead of a+b
. The standard only specifies functionality. (In fact I find it rather unfortunate that they suggest using number operators, such as <
, for set operations.)
Co-Authored-By: Kolaru <kolaru@hotmail.com>
Superseeded by #271. |
Edit: This PR has been superseeded by #271
Here is a very restricted implementation of a solution to #237.
This PR defines 3 new types of intervals, all wrapping an internal
BaseInterval
:SetInterval
is not a subtype ofReal
and error on ambiguous cases.RealSetInterval
is the same as the previous, but is a subtype ofReal
.PseudoNumberInterval
is a subtype ofReal
and returnfalse
on ambiguous cases.BaseInterval
is equivalent to the currentInterval
except for function returning boolean, for which it uses three valued logic, returning[true, false]
on ambiguous cases as bothtrue
andfalse
are possible. The wrappers then implement different response to this.A macro
@interval_function
is also implemented such that it automatically extends functions defined forBaseInterval
to the three wrappers types with the correct behavior.In the current state of this PR only the functions
lo
,hi
,in
andiszero
are available to illustrate the intended behavior. These functions are defined at the end of thewrapper_intervals.jl
file.Most of the package has been commented out to have it testable without having to do all the work before getting some review.
Here is an example of how it works:
(The error message is not yet finished, hence the "# TODO" tag)
If the community judges it is the way to go, I'll come up with a todo list for the (massive) overhaul this would imply for the package.
cc @gwater @lbenet @dpsanders