On 9/30/13, Joseph Rushton Wakeling <joseph.wakeling@webdrake.net> wrote:
> Anyway,
> best wishes to you too for a speedy recovery!

You too! I'll be fine though, 'twas just a scratch[1].

[1] : http://www.youtube.com/watch?v=zKhEw7nD9C4

On 29/09/13 20:49, Walter Bright wrote:
> On 9/28/2013 4:11 AM, Joseph Rushton Wakeling wrote:
>> I feel more like a cheerleader than a champion -- David did all the hard work!
>> ;-)
>
> The champion isn't always the same as the person who does the hardest work on it.

Hey, allow me to live the dream here.  I mean, hasn't everyone always secretly wanted to code wearing a tiny sports dress with pom-poms in hand? :-)

Joseph Rushton Wakeling:

> Anyway, any preferences?  Fix std.numeric.gcd to support BigInt and kill std.rational.gcf, or fix std.rational.gcf?

I prefer std.numeric.gcd to support BigInt and remove std.rational.gcf:
http://d.puremagic.com/issues/show_bug.cgi?id=7102

Bye,
bearophile

On 27/09/13 20:20, Walter Bright wrote:
> On 9/27/2013 6:47 AM, Joseph Rushton Wakeling wrote:
>> [...]
>
> Sounds like we have a new champion! Thanks, Joseph!

I've opened up issue tracking on my fork of std.rational:
https://github.com/WebDrake/Rational/issues

This includes the greatest-common-factor bug discovered by MrSmith and others that I've identified on detailed consideration of the code.  Some are more "need to check out how this works" rather  than "it's a problem" style issues.

Quite a few issues arise out of the fact that David Simcha obviously wanted the rational type to work effectively with any well-defined integer type of any degree of precision -- from the built-in types to std.bigint.BigInt to any arbitrary user-defined integer types.  (I can imagine that David must have been anticipating that people might want to use GMP, for example, or to hook into languages that inherently support arbitrary-precision arithmetic, like Common Lisp, Haskell or Scheme.)

Most Phobos functions don't have such generic support, so David was forced to roll his own solutions, including some templates (isIntegerLike, CommonInteger) to handle these use-cases.  In one case (his least-common-multiple function) there is in any case no Phobos equivalent that I'm aware of.

It seems to me that the best way to handle these issues in the long term is to ensure that Phobos' existing functions are sufficiently generic to handle what std.rational needs.  However, it'd be nice to get a working std.rational in place as soon as possible.

So -- what do people advise I do?  Take what I can out of std.rational and submit Phobos patches, or patch it up as it is and submit std.rational for review, with the review process deciding what stays in and what needs to be farmed out to other parts of Phobos?

On 30/09/13 14:06, bearophile wrote:
> Joseph Rushton Wakeling:
>
>> Anyway, any preferences?  Fix std.numeric.gcd to support BigInt and kill
>> std.rational.gcf, or fix std.rational.gcf?
>
> I prefer std.numeric.gcd to support BigInt and remove std.rational.gcf:
> http://d.puremagic.com/issues/show_bug.cgi?id=7102

Yes, I have your requests in mind:
https://github.com/WebDrake/Rational/issues/2

On Monday, 30 September 2013 at 12:14:10 UTC, Joseph Rushton Wakeling wrote:
> On 27/09/13 20:20, Walter Bright wrote:
>> On 9/27/2013 6:47 AM, Joseph Rushton Wakeling wrote:
>>> [...]
>>
>> Sounds like we have a new champion! Thanks, Joseph!
>
> I've opened up issue tracking on my fork of std.rational:
> https://github.com/WebDrake/Rational/issues
>
> This includes the greatest-common-factor bug discovered by MrSmith and others that I've identified on detailed consideration of the code.  Some are more "need to check out how this works" rather  than "it's a problem" style issues.
>
> Quite a few issues arise out of the fact that David Simcha obviously wanted the rational type to work effectively with any well-defined integer type of any degree of precision -- from the built-in types to std.bigint.BigInt to any arbitrary user-defined integer types.  (I can imagine that David must have been anticipating that people might want to use GMP, for example, or to hook into languages that inherently support arbitrary-precision arithmetic, like Common Lisp, Haskell or Scheme.)
>
> Most Phobos functions don't have such generic support, so David was forced to roll his own solutions, including some templates (isIntegerLike, CommonInteger) to handle these use-cases.  In one case (his least-common-multiple function) there is in any case no Phobos equivalent that I'm aware of.
>
> It seems to me that the best way to handle these issues in the long term is to ensure that Phobos' existing functions are sufficiently generic to handle what std.rational needs.  However, it'd be nice to get a working std.rational in place as soon as possible.
>
> So -- what do people advise I do?  Take what I can out of std.rational and submit Phobos patches, or patch it up as it is and submit std.rational for review, with the review process deciding what stays in and what needs to be farmed out to other parts of Phobos?

Here is my thoughts:
1) instead of gcf use updated std.numeric.gcd. Replace lcm, iAbs, floor, ceil, round functions with appropriate analogues from phobos.
2) instead of isIntegerLike use similar template from std.traits (like isIntegral).
3) replace isAssignable by std.traits.isAssignable or better use std.traits.isImplicitlyConvertible. Now i am unable to assign integer directly to rational, instead i am forced to use rational(integer) method;
4) replace CommonInteger by std.traits.CommonType

On 30/09/13 15:34, MrSmith wrote:
> Here is my thoughts:
> 1) instead of gcf use updated std.numeric.gcd. Replace lcm, iAbs, floor, ceil,
> round functions with appropriate analogues from phobos.

There is no equivalent to lcm in Phobos that I'm aware of.  floor, ceil and round -- these are type-specialized versions of generic functions and I'm not sure in this case whether the norm is for them to accompany the type definition or not.

std.numeric.gcd is currently inadequate for use with BigInt -- it could and should be patched but as a short-term workaround it may be preferable to have a private std.rational.gcf that works.

> 2) instead of isIntegerLike use similar template from std.traits (like isIntegral).

isIntegral is inadequate for purpose here because it only checks if a type is one of the built-in integer types -- it doesn't count BigInt as integral (although perhaps it should) and won't work with other arbitrary integer-like types if that's desired.

> 3) replace isAssignable by std.traits.isAssignable or better use
> std.traits.isImplicitlyConvertible. Now i am unable to assign integer directly
> to rational, instead i am forced to use rational(integer) method;

OK, I'll look into that.

> 4) replace CommonInteger by std.traits.CommonType

Not sure that's adequate -- will it handle the case of BigInt?

On 28/09/13 15:56, MrSmith wrote:
> This code fails:
>
> rational(0, 1) * rational(1, 1);
>
> It seems to me like gcf is not properly implemented. Also why its name is gcf
> (greatest common factor) and not gcd (greatest common divisor)?

I've fixed this with the following commit:
https://github.com/WebDrake/Rational/commit/6667c835af896325ccd588c3f87f9c188f359b2e

... although the new gcf will fail if passed immutable/const BigInts.

On 30/09/13 19:39, Joseph Rushton Wakeling wrote:
> ... although the new gcf will fail if passed immutable/const BigInts.

On this note: http://d.puremagic.com/issues/show_bug.cgi?id=11148

I had to tweak CommonInteger to handle immutable/const BigInts, and I'm not sure whether this was done correctly, although it seems to be largely in line with how CommonType works.

On Monday, 30 September 2013 at 18:01:29 UTC, Joseph Rushton Wakeling wrote:
> On 30/09/13 19:39, Joseph Rushton Wakeling wrote:
>> ... although the new gcf will fail if passed immutable/const BigInts.
>
> On this note: http://d.puremagic.com/issues/show_bug.cgi?id=11148
>
> I had to tweak CommonInteger to handle immutable/const BigInts, and I'm not sure whether this was done correctly, although it seems to be largely in line with how CommonType works.

Note that denumerator must be allways positive (> 0).
But numerator can be positive, zero or negative, and can be bigger than denumerator.
It means that we can use different types for numerator and denumerator:

Rational!(int, uint) r;
Rational!(long, ulong) r;
Rational!(long, uint) r;
Rational!(BigInt, BigUint) r;

We have BigUint in Phobos, but it's private.

Also, if we need only positive rationals, we can use unsigned types for numerator:

Rational!(uint, uint) r;
Rational!(ulong, ulong) r;
Rational!(ulong, uint) r;
Rational!(BigUint, BigUint) r;