Added check that an Interval is an Interval; improved readability

[alberto-paparella]

Pull request relative to issue #47, adding a check in the Interval constructor ensuring it is an interval (i.e., x<y), and improving code readability.

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 54.26%. Comparing base (d92b61a) to head (459d7af).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/utils/frames/worlds/geometrical-worlds.jl	60.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
- Coverage   54.26%   54.26%   -0.01%     
==========================================
  Files          59       59              
  Lines        3293     3319      +26     
==========================================
+ Hits         1787     1801      +14     
- Misses       1506     1518      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[giopaglia]

Actually, except during testing, I don't think we want the check, as it would probably affect performances all over the place? Maybe we should verify this by benchmarking ModalDecisionTrees.jl.

[alberto-paparella]

It's true that in automated (e.g., learning) tasks, the check is not needed and may introduce a little overhead.

However, for a general usage outside learning (e.g., wanting to know if a formula is true at a specific interval), this is no longer true, and the user can insert an interval which is, in fact, not an interval (e.g., (x,x), or (x, y) where x>y).

Not only, this would be useful for reasoning tasks, as checking that two points x and y form an interval (i.e., x<y) is still required in any case, but I'd prefer this kind of logic to live here (i.e., if I'm calling a constructor for Interval elsewhere, I expect it to fail if I give it two points such that they do not form an interval, and not having to check it myself; even more so if an external user is defining a new method accepting an interval).

At the same time, I noticed that checking that x > 0 breaks the emptyworld function, which for Interval defines an interval (-1,0). For the moment, I can fix it requiring that x > -1, but we should specify clear documentation for the user informing that linear orders (in the current implementation) start from 1 and (-1,0) is used to refer to an empty world.

[giopaglia]

Still I believe the overhead that this check brings is not justified.
What if we differentiate the two needs, and have a constructor that does the check (for consistency checking from user input, and another that does not (to be used for speed)?
We could have a function check_interval_is_consistent() that one of the two constructor calls.
By the way, let me note that a lot of opportunities open up if we also consider oriented intervals, where x < y does not hold necessarily. The existing IA relations will be easily adjusted to that case.

As for the empty world; I wish there was a way to avoid using it, but I think we have to keep it. Note that the empty world is never read, it is a placeholder, just to have a non-empty vector in MDTs.jl: [Interval(-1, 0)], instead of []. It could be replaced with any interval in fact. So an easy way to uniform it to your boundary check, is to have it be another interval, like Interval(0, 1) or Interval(1, 2). This will have no effect, and will be more consistent with your checks.

[alberto-paparella]

I agree that differentiating the two needs might be the right solution.

Since as you say in the future we might consider oriented intervals, I suggest having an abstract type AbstractInterval that both the current Interval and, e.g., OrientedInterval might call, as most functions defined for Interval in fact work for any I<:Interval.

I also propose renaming the current Interval to something else, implying that no check is performed at the moment and which will be used for internal operations we have control over, like learning tasks, and keeping Interval with checks as the gateway for the user.

Regarding emptyworld, since apparently at the moment the first Interval of a FullDimensionalFrame{1,Interval{Int}} is (1,2) (in fact, every FullDimensionalFrame start with 1 on either axes), using (1,2) would probably be the best solution.

However, since this solution requires more work than expected, I'll leave it open for the moment for whoever wants to approach the task, giving priority to more urgent issues.

[giopaglia]

I think I have the solution regarding the x < y check! we could label that check with @boundscheck. That makes sure that that code is executed only when the executed Julia code doesNot have @inbounds. So we can retain speed just by running @inbounds Interval(...) or @inbounds {any Interval-constructing code}. For example, @inbounds ModalDecisionTrees.build_tree(...) will act as it does now.