Unify the classes RealSet and UnionOfIntervals

[pjbruin]

The following two classes were introduced almost simultaneously:

• sage.sets.real_set.RealSet in Reals sets consisting of intervals and isolated points #13125, which implements finite unions of open/closed/semi-closed intervals;
• sage.schemes.elliptic_curves.height.UnionOfIntervals in Lower height bound for elliptic curves #8828, which has a more restricted scope (only unions of closed intervals).
  It would be reasonable if these two classes were merged, and if all the functionality of UnionOfIntervals that is not yet in RealSet were moved there (e.g. scaling, computing the endpoints/boundary).

Depends on #8828
Depends on #13125

CC: @sagetrac-ares @JohnCremona @robertwb @vbraun @rwst @tscrim

Component: calculus

Keywords: real intervals

Author: Matthias Koeppe

Branch/Commit: 2191e2a

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/16063

[mkoeppe]

Branch: u/mkoeppe/unify_the_classes_realset_and_unionofintervals

[mkoeppe]

New commits:

63f2fc9

RealSet, InternalRealInterval: Multiplication scales

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Commit: 63f2fc9

[mkoeppe]

comment:5

Volker: Would you have a moment to take a look whether I'm on the right track here.
I'm wondering why the normalization of the init arguments of InternalRealInterval are not done by a method InternalRealInterval.__classcall__ but rather by RealSet._prep. Because of this I have to duplicate some of this normalization in InternalRealInterval.__mul__.

[tscrim]

comment:8

Looking briefly at the code, UnionOfIntervals is somewhat more specialized, and so it is likely is faster than the more generic (and [indirectly] in the global namespace) InternalRealInterval. I'm not convinced that we should remove UnionOfIntervals unless there is not a speed regression (or it is minor).

However, I am +1 for unifying the behavior of these two classes. If you feel that you are having to duplicate some of the normalization code, then it might be better to try and refactor it because of the new use-case you have.

[JohnCremona]

comment:9

Replying to @tscrim:

Looking briefly at the code, UnionOfIntervals is somewhat more specialized, and so it is likely is faster than the more generic (and [indirectly] in the global namespace) InternalRealInterval. I'm not convinced that we should remove UnionOfIntervals unless there is not a speed regression (or it is minor).

+1 The UnionOfIntervals was written for one specific application where timing is very important.

However, I am +1 for unifying the behavior of these two classes. If you feel that you are having to duplicate some of the normalization code, then it might be better to try and refactor it because of the new use-case you have.

[mkoeppe]

comment:10

Replying to @JohnCremona:

Replying to @tscrim:

Looking briefly at the code, UnionOfIntervals is somewhat more specialized, and so it is likely is faster than the more generic (and [indirectly] in the global namespace) InternalRealInterval. I'm not convinced that we should remove UnionOfIntervals unless there is not a speed regression (or it is minor).

+1 The UnionOfIntervals was written for one specific application where timing is very important.

Do you have a benchmark that I can run without having to know anything about your application?