std.rational -- update and progress towards review

[Joseph Rushton Wakeling]

On 02/10/13 19:37, H. S. Teoh wrote:
>>     * Because std.rational doesn't just want to work with built-in
>>       integer types it can't rely on the existing isIntegral.
>
> TBH, I found isIntegral rather counterintuitive. I thought it would
> evaluate to true with BigInt, but it doesn't. If I had my way, I'd
> propose renaming isIntegral to isBuiltInIntegral, and David's
> isIntegerLike to isIntegral.

It's a fair thought, but at this point I guess we have to consider whether 
people may be using isIntegral specifically to check for built-in integer type. 
  (I seem to remember someone -- Bearophile? -- filed an enhancement request for 
isIntegral to expand its scope to include BigInts, but searching D bugzilla now 
I can't find it.  Maybe it was just a forum discussion.)

> Maybe we could have a bit of discussion here in the forum on all of the
> proposed additions to std.traits, then once that decision is made, put
> up std.rational for the actual "official" review?

That's what I was hoping for -- get all the "what goes where" decisions out of 
the way first, then there's less to worry about in the official review.

> Since Rational is a templated type, no function attributes are needed.
> The compiler should be able to infer the appropriate attributes.
>
> Unless, of course, you find a case where it makes sense to constrict any
> future changes in implementation (e.g., guarantee that gcd is always
> pure -- but even that is questionable since gcd's purity would depend on
> the purity of operations on the type it is being instantiated for, so
> even in this case I'd say keep it unmarked and let attribute inference
> do its job).

Ahh, OK.  I don't feel 100% on what the compiler can infer attribute-wise 
compared to what needs to be explicitly written.

>> The remaining open issues
>> <https://github.com/WebDrake/Rational/issues?state=open> are all
>> design-related.  Apart from those raised by my above queries, the
>> major one is how rationals should relate to floating-point numbers --
>> e.g. there is currently no opCmp for floating-point, meaning this:
>
>>
>>      assert(rational(10, 1) == 10);
>>
>> ... will work, but this:
>>
>>      assert(rational(10, 1) == 10.0);
>>
>> ... will fail to compile.  It's not entirely obvious how to resolve
>> this as floating-point vs. rational comparisons risk accidentally
>> creating huge temporary BigInt-based rationals ... :-(
>
> Does addition/subtraction with floating point work correctly? If so, the
> user should simply write:
>
> 	assert(abs(rational(10,1) - 10.0) < EPSILON);

Well, yes, obviously one can use this formalism.  On that note, approxEqual 
won't work with rationals, e.g.:

     auto r1 = rational(10);
     assert(approxEqual(r1, 10.0));

fails with error message:

     /opt/dmd/include/d2/std/math.d(5689): Error: function std.math.fabs (real 
x) is not callable using argument types (Rational!int)
     /opt/dmd/include/d2/std/math.d(5696): Error: incompatible types for ((lhs) 
- (rhs)): 'Rational!int' and 'double'
     /opt/dmd/include/d2/std/math.d(5697): Error: incompatible types for ((lhs) 
- (rhs)): 'Rational!int' and 'double'
     /opt/dmd/include/d2/std/math.d(5707): Error: template instance 
std.math.approxEqual!(Rational!int, double, double) error instantiating
     rational.d(57):        instantiated from here: approxEqual!(Rational!int, 
double)
     rational.d(57): Error: template instance 
std.math.approxEqual!(Rational!int, double) error instantiating

(Ignore the line number, it's a temporary unittest I knocked up just to try this 
out just now and is not in the repo.)

You can do approxEqual(cast(real) r1, float1), however.

> We should definitely not convert floats to Rational just so they can be
> compared, because floats are inexact by definition, whereas Rationals
> are always exact. For example, rational(2,10) != 0.2f, because 0.2f has
> no exact representation in any binary floating-point format. But if you
> support rational(10,1) == 10.0, then people will expect that
> rational(2,10) == 0.2 should also work, but it *can't* work.  We should
> not sweep these issues under the rug, but force the user to come to
> terms with the nature of floating-point numbers.

Well, the point is that anyone who knows anything about floating point knows 
that comparisons of the form float1 == float2 or float1 == int1 are dangerous 
because tiny rounding errors can result in the floating-point number being ever 
so slightly off.  But you're not _banned_ from making the comparison; the code 
won't fail to compile.

So, it feels bad that there isn't an opCmp for floating-point, even though I can 
see logical reasons for that.  After all, it's one thing that you can't 
guarantee an opEquals, another that you can't do something like

     auto r1 = rational(2, 3);
     assert(r1 < 0.8);

> OTOH, one compromise might be to allow implicit conversion of Rationals
> to floating-point via alias this:
>
> 	struct Rational(T) {
> 		...
> 		@property real toReal() { return this.convertToReal(); }
> 		alias toReal this;
> 	}
>
> 	void main() {
> 		float x = 1.0;
> 		assert(rational(1,1) == x);
> 	}

There is already an opCast for floating point, so you could define an opEquals 
that does something like:

     int opEquals(Rhs)(Rhs rhs)
         if (isFloatingPoint!Rhs)
     {
         return (cast(real) this) == rhs;
     }

(Ad-hoc knock-up here for example purposes, have not tried or tested:-)

> This works because Rational implicitly converts to real, and x also
> implicitly converts to real, and both are then comparable.

The trouble is that this is ultimately selling the user a false promise, because 
the cast to real is in general approximating rather than equalling the rational 
number. :-(