[phobos] Rational for review

[David Simcha]

  Yes please what?  isAssignable?  The whole module?  I'm confused.

On 8/28/2010 6:07 PM, Andrei Alexandrescu wrote:
> 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
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
>