[phobos] Rational for review

[Andrei Alexandrescu]

Yes please.

Sent by shouting through my showerhead.

On Aug 28, 2010, at 4:22 PM, David Simcha <dsimcha at gmail.com> wrote:

> On 8/28/2010 4:22 PM, Philippe Sigaud wrote:
>>
>> line 76: isAssignable would be a good addition to std.traits.
>
> Yeah, will do unless anyone has any objections.
>
>>
>>
>>>
>>> Next, 1/0. OK, it's correctly dealt with (Integer Divide By Zero  
>>> Error)
>>> But 0/1 botches. In fact, any rational that's zero creates a DBZ  
>>> error. r-= r, etc.
>>> I guess it's a pb in simplify (454-455) or gcf (638), you should  
>>> test for a zero numerator or divisor
>>
>> I can't believe I forgot to test this edge case.  Fixed by simply  
>> testing for zero as a special case in simplify().
>>
>> Put in some very basic unit tests :-)  0+0==0, 1-1==0, that kind of  
>> thing :)
>
> Did that already.  They pass.
>
>>
>>>
>>> Now, for the code itself:
>>> line 35: Maybe you should add %, %=, they are part of what I  
>>> consider 'integer like'. You use them in gcf anyway
>>
>> Good point.  Done.  Also added comparison.
>>
>> Man, integer-like is beginning to be quite big.
>>
>> What about adding ++ and -- ?
>
> I don't need/use them, so there's no real reason to require them.
>
>>
>> Btw, I'm against putting ++ and -- in Rational, but please do add  
>> opUnary!"+", opUnary!"-".
>
> Will do, along with ^^.  These were left out as oversights, not for  
> "good" reasons.
>
>>
>>> * a fractional part function (ie 22/7 is ~3.14 -> 0.14) as a FP
>>
>> Ditto.
>>
>> Thinking about it some more, maybe the fractional part should  
>> return a typeof(this): 22/7 -> 1/7. Not a float.
>
> Yea, this makes more sense.  If you want a float, you can always  
> cast it.
>
>>
>>> * Accordingly,  floor/ceil functions could be interesting, unless  
>>> you think the user should use std.math.floor(cast(real)rat), which  
>>> I could understand.
>>
>> Again, good idea.  Using std.math sucks because a major point of  
>> Rational is that it's supposed to work with arbitrary precision,  
>> and reals aren't arbitrary precision.
>>
>>
>> I could argue that floor or ceil are exactly when you get rid of  
>> this arbitrary precision, but to answer the question in your next  
>> post, I think these should be part of the rational module and be  
>> free functions: it makes the module self-supported. And having  
>> rational support in std math means it must in turn import the  
>> rational module. I'm against this.
>>
>> In any case, people will probably import both at the same time...
>>
>> I'm not clear on the subtle nuances between the integer part, the  
>> floor and the ceil for positive and negative Rationals, though.
>>
>> See std.math: ceil, floor, nearbyInt, round, trunc! Awww.
>>
>> The next question would be: what other free function? I'll add abs 
>> (), but not much more. Maybe pow or opBinary!"^^" ?
>
> opBinary!"^^":  Yes.  pow():  I don't see the point.  Probably  
> everyone writing generic math code nowadays will use ^^ instead of  
> pow() because it's less typing and more readable.  Then again, pow()  
> would be trivial to implement in terms of opBinary!"^^".
>
>> I wouldn't add any trigonometric function.
>
>> .
>> sqrt is on the fence for me. Is there an algorithm to calculate the  
>> square root of a rational apart from the Babylonian method?
>
> I'm going to leave trig and sqrt out.  To me, the point of rationals  
> is to allow calculations to be exact.  Functions where the result  
> can be irrational don't make sense in this context.  You'd be better  
> off just casting to real and then calling the std.math functions.
>
>>
>>> ***
>>>  2.  I couldn't figure out how to handle the case of user-defined  
>>> fixed-width integer types in the decimal conversion (opCast)  
>>> function, so it just assumes non-builtin integer types are  
>>> arbitrary                     precision.  The biggest problem is  
>>> lack of a way of introspecting whether the integer is fixed or  
>>> arbitrary width and what its fixed width might be if it is, in  
>>> fact, fixed.  The secondary problem is                     that I  
>>> don't know of a good algorithm (though I'm sure I could find one  
>>> if I tried) for converting fixed-width integers to floating point  
>>> numbers in software.  Arbitrary precision is much easier.
>>> ***
>>>
>>> About converting fixed-width integer-like types, maybe these  
>>> should provide .min and .max properties?
>>> That way you know the range of possible values and maybe it's then  
>>> possible to convert (I didn't think this through so I may be  
>>> completly mistaken)
>>
>> I think what I'm going to do here is just throw an exception iff  
>> the denominator has both the highest and lowest order bit set.   
>> This is enough of a corner case in that it will only happen close  
>> to the limit of the largest denominator value that can be  
>> represented that I don't consider it a major limitation.  It's  
>> easily detectable because the numerator will never become bigger  
>> than the denominator by bit shifting iff either the numerator is  
>> zero or the denominator has its highest order bit set.  If the  
>> denominator's lowest order bit isn't set then I can shift the  
>> denominator right to make it not have its highest order bit set.
>>
>> What's the pb with having a numerator bigger than the denominator?  
>> Sorry, I didn't read the conversion function carefully.
>
> The way the function works involves bit shifting the numerator left  
> until it's bigger than the denominator.  If the denominator has its  
> highest order bit occupied, no amount of shifting will ever make the  
> numerator bigger.
>>
>> This code has been around in some form for almost a year.  It's  
>> just that the proposal for a units library made me remember that  
>> I've been meaning to clean up all the bit rot, improve it and  
>> submit it for inclusion.
>>
>> Seeing that you have time, talent and energy, I'll order a  
>> BigDecimal module :-)
>
> I'd love one, but I haven't the slightest clue how to write one and  
> have several hacking projects that I need to finish/start before  
> even considering it.
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos