www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.rational?

reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
Does anybody happen to have a working rational number library in D? Any
plans to add that to Phobos at some point?

Just wondering, because I'm working on some numerical code that would
benefit from a custom number type that could be implemented in terms of
rational numbers. I just don't feel like reinventing a rational library
if one already exists. :)


T

-- 
GEEK = Gatherer of Extremely Enlightening Knowledge
Sep 25 2013
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Wednesday, 25 September 2013 at 16:59:15 UTC, H. S. Teoh wrote:

 Does anybody happen to have a working rational number library 
 in D?


https://github.com/p0nce/gfm/blob/master/gfm/math/fraction.d

disclaimer: I've never used it for anything except writing its 
unittest block.
Sep 25 2013
parent reply "ponce" <contact gam3sfrommars.fr> writes:
On Wednesday, 25 September 2013 at 17:15:25 UTC, ponce wrote:

 On Wednesday, 25 September 2013 at 16:59:15 UTC, H. S. Teoh 
 wrote:

 Does anybody happen to have a working rational number library 
 in D?




Also David Simcha wrote a better one: 
https://github.com/dsimcha/Rational/blob/master/rational.d
Sep 25 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 25/09/13 19:21, ponce wrote:

 On Wednesday, 25 September 2013 at 17:15:25 UTC, ponce wrote:

 On Wednesday, 25 September 2013 at 16:59:15 UTC, H. S. Teoh wrote:

 Does anybody happen to have a working rational number library in D?




 Also David Simcha wrote a better one:
 https://github.com/dsimcha/Rational/blob/master/rational.d


Why was this never incorporated into Phobos?
Sep 26 2013
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Joseph Rushton Wakeling:


 Why was this never incorporated into Phobos?


First it needs the normal review process for Phobos modules :) It 
should go in the review queue, if it's good enough for Phobos.


 Rational!(BigInt) getTerm(int termNumber) {


I suggest to add inside that "rational.d" module an alias with 
Bigint, as:

alias Fraction = Rational!BigInt;

Because I don't think I want to use a rational type made with 
built-in integrals.

Bye,
bearophile
Sep 26 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 26/09/13 16:09, bearophile wrote:

 First it needs the normal review process for Phobos modules :) It should go in
 the review queue, if it's good enough for Phobos.


Fair enough, I was wondering if there was any reason David never submitted it.


 I suggest to add inside that "rational.d" module an alias with Bigint, as:

 alias Fraction = Rational!BigInt;

 Because I don't think I want to use a rational type made with built-in
integrals.


Good call.
Sep 26 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 9/26/2013 12:49 PM, Joseph Rushton Wakeling wrote:

 On 26/09/13 16:09, bearophile wrote:

 First it needs the normal review process for Phobos modules :) It should go in
 the review queue, if it's good enough for Phobos.


 Fair enough, I was wondering if there was any reason David never submitted it.


For things to happen, usually a self-appointed champion is needed that will 
relentless push it forward. Things tend to fall by the wayside otherwise.
Sep 27 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 27/09/13 09:36, Walter Bright wrote:

 For things to happen, usually a self-appointed champion is needed that will
 relentless push it forward. Things tend to fall by the wayside otherwise.


In the past David has seemed like a pretty reliable champion of his own work -- 
quite a few of the parts of Phobos I use regularly are down to him!

I've written to him asking for his input on std.rational but if he doesn't have 
time/inclination to pursue it, I'll take a look at it.

At first glance what's there looks pretty good with the main issues being 
stylistic rather than content-related.
Sep 27 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Friday, 27 September 2013 at 10:11:47 UTC, Joseph Rushton 
Wakeling wrote:

 On 27/09/13 09:36, Walter Bright wrote:

 For things to happen, usually a self-appointed champion is 
 needed that will
 relentless push it forward. Things tend to fall by the wayside 
 otherwise.


 In the past David has seemed like a pretty reliable champion of 
 his own work -- quite a few of the parts of Phobos I use 
 regularly are down to him!

 I've written to him asking for his input on std.rational but if 
 he doesn't have time/inclination to pursue it, I'll take a look 
 at it.

 At first glance what's there looks pretty good with the main 
 issues being stylistic rather than content-related.


As usual, volunteering to act as review manager. Once you are 
ready to championship this module, add it to review queue in wiki 
and ping me (preferably via e-mail)
Sep 27 2013
next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 27/09/13 12:37, Dicebot wrote:

 As usual, volunteering to act as review manager. Once you are ready to
 championship this module, add it to review queue in wiki and ping me
(preferably
 via e-mail)


Thanks very much! :-)

I've made a fork <https://github.com/WebDrake/Rational> and tweaked the code to 
be in line with current D/Phobos style (David Simcha's last update is from over 
2 years ago so it's out of date in that respect).  I did this manually, line by 
line, so I could have a proper read-through of the code.

On the basis of that read-through it looks pretty comprehensive and 
well-thought-out.  There are various workarounds for bugs which probably need
to 
be re-examined depending on whether those bugs have been fixed or not.  In any 
case it compiles fine with current DMD, and the unittests are thorough and have 
94% code coverage -- good job, David!

One thing I don't like: David uses "num" and "denom" for the public methods to 
get numerator and denominator.  I think it should be "numerator" and 
"denominator".  This matches Boost.Rational but I don't know if David was
naming 
these methods by analogy to some other library/language.  In any case I should 
probably make a detailed comparison to Boost.Rational; I don't know if this was 
David's model or whether this is a clean-room implementation.

There are also probably some Ddoc tweaks that need to be made, not everything 
seems to be perfectly in order on my system.

Anyway, if anyone else wants to take a look through and give me any comments, 
they'd be welcome.  (Bearophile, I will add your requested Fraction, but I'd 
like to get more info on what everyone else thinks of the code as it is before 
making any additions:-)

Best wishes,

     -- Joe
Sep 27 2013
next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
27-Sep-2013 17:47, Joseph Rushton Wakeling пишет:

 On 27/09/13 12:37, Dicebot wrote:


[snip]

 One thing I don't like: David uses "num" and "denom" for the public
 methods to get numerator and denominator.  I think it should be
 "numerator" and "denominator".  This matches Boost.Rational but I don't
 know if David was naming these methods by analogy to some other
 library/language.


I bet the reason is practicality: try using full names of
denominator/numerator in some involved numeric code. It's a mess.
One may argue you need not access numerator and denominator explicitly 
that much but I think it happens.

-- 
Dmitry Olshansky
Sep 27 2013
next sibling parent reply "BLM768" <blm768 gmail.com> writes:
On Friday, 27 September 2013 at 15:36:12 UTC, Dmitry Olshansky 
wrote:

 I bet the reason is practicality: try using full names of
 denominator/numerator in some involved numeric code. It's a 
 mess.
 One may argue you need not access numerator and denominator 
 explicitly that much but I think it happens.


I'm not sure if this fits with Phobos style, but they could be 
defined as "numerator" and "denominator" and aliased to "num" and 
"denom", respectively. That way, we get the best of both worlds.
Sep 27 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/27/13 1:08 PM, BLM768 wrote:

 On Friday, 27 September 2013 at 15:36:12 UTC, Dmitry Olshansky wrote:

 I bet the reason is practicality: try using full names of
 denominator/numerator in some involved numeric code. It's a mess.
 One may argue you need not access numerator and denominator explicitly
 that much but I think it happens.


 I'm not sure if this fits with Phobos style, but they could be defined
 as "numerator" and "denominator" and aliased to "num" and "denom",
 respectively. That way, we get the best of both worlds.


No aliases please. num and dom is where it's at.

Andrei
Sep 27 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 9/27/2013 1:25 PM, Andrei Alexandrescu wrote:

 No aliases please.


I agree. Aliases are not a solution to bikeshedding about names and inability
to 
reach a consensus.
Sep 27 2013
parent "eles" <eles eles.com> writes:
On Friday, 27 September 2013 at 22:20:42 UTC, Walter Bright wrote:

 On 9/27/2013 1:25 PM, Andrei Alexandrescu wrote:

 No aliases please.


 I agree. Aliases are not a solution to bikeshedding about names 
 and inability to reach a consensus.


what about std.uni => std.unicode?
Sep 28 2013
prev sibling next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 27/09/13 17:36, Dmitry Olshansky wrote:

 I bet the reason is practicality: try using full names of
 denominator/numerator in some involved numeric code. It's a mess.


Well, obviously.  Actually I'm not unhappy with David's name per se but with
the 
idea of being inconsistent with how other libraries name these methods. 
Boost.Rational uses the full names, I'll have a look around and see how other 
libraries name them and whether there is consensus.

Next week I'll try and do a close comparison of Boost.Rational and David's work 
to see if there's any overlap, at least in the core data type.

Is there any expectation that some later iteration of the C++ is going to 
specify a rational type?  If so, any expected naming convention?  I guess Boost 
reflects what would be implemented if so, but it's never an absolute, things in 
Boost.Random were dropped in the C++11 standard.


 One may argue you need not access numerator and denominator explicitly that
much
 but I think it happens.


I agree.
Sep 28 2013
parent reply "eles" <eles eles.com> writes:
On Saturday, 28 September 2013 at 11:16:03 UTC, Joseph Rushton 
Wakeling wrote:

 On 27/09/13 17:36, Dmitry Olshansky wrote:

 I bet the reason is practicality: try using full names of
 denominator/numerator in some involved numeric code. It's a 
 mess.




up/down?
above/below?
hi/lo?
Sep 28 2013
parent reply "MrSmith" <mrsmith33 yandex.ru> writes:
This code fails:

rational(0, 1) * rational(1, 1);


object.Error: Integer Divide by Zero
----------------
0x00408228 in int std.rational.gcf!(int, int).gcf(int, int) at 
C:\rational\rational.d(878)
0x0040820A in int std.rational.gcf!(int, int).gcf(int, int) at 
C:\rational\rational.d(870)
0x004082E7 in std.rational.Rational!(int).Rational 
std.rational.Rational!(int).Rational.opOpAssign!("*", 
std.rational.Rational!(int).Rational).opOpAssign(std.rational.Rati
nal!(int).Rational) 
at C:\rational\rational.d(235)
0x004087C3 in std.rational.Rational!(int).Rational 
std.rational.Rational!(int).Rational.opBinary!("*", 
std.rational.Rational!(int).Rational).opBinary(std.rational.Rati
nal!(int).Rational) 
at C:\rational\rational.d(218)
0x0040880C in _Dmain at C:\rational\test.d(8)
0x0040C7EC in extern (C) int rt.dmain2._d_run_main(int, char**, 
extern (C) int function(char[][])*).void runMain()
0x0040C827 in extern (C) int rt.dmain2._d_run_main(int, char**, 
extern (C) int function(char[][])*).void runAll()
0x0040C425 in _d_run_main
0x0040A064 in main
0x769C33AA in BaseThreadInitThunk
0x77C19EF2 in RtlInitializeExceptionChain
0x77C19EC5 in RtlInitializeExceptionChain
----------------

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)?
Sep 28 2013
next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/09/13 15:56, MrSmith wrote:

 This code fails:

 rational(0, 1) * rational(1, 1);


 object.Error: Integer Divide by Zero
 ----------------
 0x00408228 in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(878)
 0x0040820A in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(870)
 0x004082E7 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opOpAssign!("*",
 std.rational.Rational!(int).Rational).opOpAssign(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(235)
 0x004087C3 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opBinary!("*",
 std.rational.Rational!(int).Rational).opBinary(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(218)
 0x0040880C in _Dmain at C:\rational\test.d(8)
 0x0040C7EC in extern (C) int rt.dmain2._d_run_main(int, char**, extern (C) int
 function(char[][])*).void runMain()
 0x0040C827 in extern (C) int rt.dmain2._d_run_main(int, char**, extern (C) int
 function(char[][])*).void runAll()
 0x0040C425 in _d_run_main
 0x0040A064 in main
 0x769C33AA in BaseThreadInitThunk
 0x77C19EF2 in RtlInitializeExceptionChain
 0x77C19EC5 in RtlInitializeExceptionChain
 ----------------

 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)?


Good catch, thank you!

What compiler and options are you using to get that detailed error report? 
With 
dmd -main -unittest -debug rational.d I get only: "Floating point exception 
(core dumped)".  (Albeit I'm on Linux not Windows, I guess that might make a 
difference.)
Sep 29 2013
next sibling parent reply "MrSmith" <mrsmith33 yandex.ru> writes:
use -debug -gc
Sep 29 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 29/09/13 16:46, MrSmith wrote:

 use -debug -gc


Nope, I still get just the "floating point exception" message.
Sep 29 2013
parent "MrSmith" <mrsmith33 yandex.ru> writes:
On Sunday, 29 September 2013 at 17:59:23 UTC, Joseph Rushton 
Wakeling wrote:

 On 29/09/13 16:46, MrSmith wrote:

 use -debug -gc


 Nope, I still get just the "floating point exception" message.


I've used for this compilation the following command:
dmd -property -debug -gc rational.d test.d -of"test.exe"

Are you catching and printing exception?
Sep 29 2013
prev sibling parent reply "John Colvin" <john.loughran.colvin gmail.com> writes:
On Sunday, 29 September 2013 at 11:03:46 UTC, Joseph Rushton 
Wakeling wrote:

 On 28/09/13 15:56, MrSmith wrote:

 This code fails:

 rational(0, 1) * rational(1, 1);


 object.Error: Integer Divide by Zero
 ----------------
 0x00408228 in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(878)
 0x0040820A in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(870)
 0x004082E7 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opOpAssign!("*",
 std.rational.Rational!(int).Rational).opOpAssign(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(235)
 0x004087C3 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opBinary!("*",
 std.rational.Rational!(int).Rational).opBinary(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(218)
 0x0040880C in _Dmain at C:\rational\test.d(8)
 0x0040C7EC in extern (C) int rt.dmain2._d_run_main(int, 
 char**, extern (C) int
 function(char[][])*).void runMain()
 0x0040C827 in extern (C) int rt.dmain2._d_run_main(int, 
 char**, extern (C) int
 function(char[][])*).void runAll()
 0x0040C425 in _d_run_main
 0x0040A064 in main
 0x769C33AA in BaseThreadInitThunk
 0x77C19EF2 in RtlInitializeExceptionChain
 0x77C19EC5 in RtlInitializeExceptionChain
 ----------------

 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)?


 Good catch, thank you!

 What compiler and options are you using to get that detailed 
 error report?  With dmd -main -unittest -debug rational.d I get 
 only: "Floating point exception (core dumped)".  (Albeit I'm on 
 Linux not Windows, I guess that might make a difference.)


It's a linux thing AFAIK
Sep 29 2013
parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Sunday, 29 September 2013 at 21:14:09 UTC, John Colvin wrote:

 On Sunday, 29 September 2013 at 11:03:46 UTC, Joseph Rushton 
 Wakeling wrote:

 On 28/09/13 15:56, MrSmith wrote:

 This code fails:

 rational(0, 1) * rational(1, 1);


 object.Error: Integer Divide by Zero
 ----------------
 0x00408228 in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(878)
 0x0040820A in int std.rational.gcf!(int, int).gcf(int, int) at
 C:\rational\rational.d(870)
 0x004082E7 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opOpAssign!("*",
 std.rational.Rational!(int).Rational).opOpAssign(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(235)
 0x004087C3 in std.rational.Rational!(int).Rational
 std.rational.Rational!(int).Rational.opBinary!("*",
 std.rational.Rational!(int).Rational).opBinary(std.rational.Rational!(int).Rational)
 at C:\rational\rational.d(218)
 0x0040880C in _Dmain at C:\rational\test.d(8)
 0x0040C7EC in extern (C) int rt.dmain2._d_run_main(int, 
 char**, extern (C) int
 function(char[][])*).void runMain()
 0x0040C827 in extern (C) int rt.dmain2._d_run_main(int, 
 char**, extern (C) int
 function(char[][])*).void runAll()
 0x0040C425 in _d_run_main
 0x0040A064 in main
 0x769C33AA in BaseThreadInitThunk
 0x77C19EF2 in RtlInitializeExceptionChain
 0x77C19EC5 in RtlInitializeExceptionChain
 ----------------

 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)?


 Good catch, thank you!

 What compiler and options are you using to get that detailed 
 error report?  With dmd -main -unittest -debug rational.d I 
 get only: "Floating point exception (core dumped)".  (Albeit 
 I'm on Linux not Windows, I guess that might make a 
 difference.)


 It's a linux thing AFAIK


i.e. windows is more user friendly in this regard.
Sep 29 2013
prev sibling next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Sep 28, 2013 at 03:56:26PM +0200, MrSmith wrote:

 This code fails:
 
 rational(0, 1) * rational(1, 1);
 
 
 object.Error: Integer Divide by Zero


[...]

https://github.com/WebDrake/Rational/pull/1


T

-- 
Bare foot: (n.) A device for locating thumb tacks on the floor.
Sep 29 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/09/13 15:56, MrSmith wrote:

 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)?


Greatest common factor is just another term for greatest common divisor.  I 
don't know why David chose it, maybe because he's a scientist and liked the
more 
traditionally formal term, maybe because gcd is typical and he wanted to avoid
a 
clash with other implementations in other modules.

Anyway, you're right, it's not properly implemented -- it can't handle the case 
of one of its inputs being 0.  In fact we should have gcf(0, n) == n.
Sep 30 2013
prev sibling next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 30/09/13 00:21, H. S. Teoh wrote:

 https://github.com/WebDrake/Rational/pull/1


Thanks, but unfortunately this pull works around the problem rather than
solving 
it :-)

As MrSmith identified, it's gcf() that is at fault because it can't handle 0 as 
an input even though it should -- gcf(0, n) == gcf(n, 0) == n.

What's odd is why David didn't go with the (apparently pre-existing) 
std.numeric.gcd.  It doesn't currently support BigInts but it could be fixed to 
do so.

IIRC the consensus is that std.numeric is broken/borked beyond repair and is 
only waiting for a valid successor before it gets deprecated.  Was this already 
clear 2 years ago when David was writing std.rational?

Anyway, any preferences?  Fix std.numeric.gcd to support BigInt and kill 
std.rational.gcf, or fix std.rational.gcf?
Sep 30 2013
parent reply "bearophile" <bearophileHUGS lycos.com> writes:
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
Sep 30 2013
parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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
Sep 30 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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.
Sep 30 2013
prev sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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.
Sep 30 2013
parent reply "ilya-stromberg" <ilya-stromberg-2009 yandex.ru> writes:
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;
Sep 30 2013
next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 01/10/13 08:26, ilya-stromberg wrote:

 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:


Seems an overcomplication for me, and it doesn't save you from denominator ==
0, 
so it doesn't get you out of any checks.


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


Interesting.  Any particular reason?  (I suppose the nature of BigInt is that 
you don't need to worry about signed/unsigned distinction because there is no 
range limit.)


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


Yes, but you can already get that as is by Rational!(UnsignedTypeOfChoice) :-)
Oct 01 2013
parent reply "ilya-stromberg" <ilya-stromberg-2009 yandex.ru> writes:
On Tuesday, 1 October 2013 at 08:02:12 UTC, Joseph Rushton 
Wakeling wrote:

 On 01/10/13 08:26, ilya-stromberg wrote:

 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:


 Seems an overcomplication for me, and it doesn't save you from 
 denominator == 0, so it doesn't get you out of any checks.


It's from a mathematics:
http://ru.wikipedia.org/wiki/%D0%94%D1%80%D0%BE%D0%B1%D1%8C_%28%D0%BC%D0%B0%D1%82%D0%B5%D0%BC%D0%B0%D1%82%D0%B8%D0%BA%D0%B0%29
Numerator must be integer (..., -2, -1, 0, 1, 2, ...):
http://en.wikipedia.org/wiki/Integer
Denumerator must be natural number (1, 2, 3, ...):
http://en.wikipedia.org/wiki/Natural_number

Note that english wikipedia says that denumerator is integer not 
equal to zero:
http://en.wikipedia.org/wiki/Rational_number
It's interesting: have we got 2 different mathematics, or it's 
just mistake in wikipedia? I didn't read any english mathematics 
books, but the GMP library agree with me:
"All rational arithmetic functions assume operands have a 
canonical form, and canonicalize their result. The canonical from 
means that the denominator and the numerator have no common 
factors, and that the denominator is positive. Zero has the 
unique representation 0/1."
http://gmplib.org/manual/Rational-Number-Functions.html#Rational-Number-Functions

May be you are rigth and store numerator and denumerator type is 
too complex, but it have some benefits. For example, we can save 
some memory with BigUint type because it doesn't store 
unnecessary sign.


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


 Interesting.  Any particular reason?  (I suppose the nature of 
 BigInt is that you don't need to worry about signed/unsigned 
 distinction because there is no range limit.)


I don't know, I just found it in source code:
https://github.com/D-Programming-Language/phobos/blob/master/std/bigint.d

module std.bigint;

private import std.internal.math.biguintcore;

struct BigInt
{
private:
         BigUint data; // BigInt adds signed arithmetic to BigUint.
         bool sign = false;
//...
}


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


 Yes, but you can already get that as is by 
 Rational!(UnsignedTypeOfChoice) :-)
Oct 01 2013
parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 01/10/13 10:35, ilya-stromberg wrote:

 It's from a mathematics:
 http://ru.wikipedia.org/wiki/%D0%94%D1%80%D0%BE%D0%B1%D1%8C_%28%D0%BC%D0%B0%D1%82%D0%B5%D0%BC%D0%B0%D1%82%D0%B8%D0%BA%D0%B0%29

 Numerator must be integer (..., -2, -1, 0, 1, 2, ...):
 http://en.wikipedia.org/wiki/Integer
 Denumerator must be natural number (1, 2, 3, ...):
 http://en.wikipedia.org/wiki/Natural_number


I'm fully aware of the maths of it.  However, bear in mind that

   (i) m/(-n) is also a rational number, and that std.rational needs to support
       that case -- it's just that by convention in maths when we have m/(-n) we
       rewrite that as -m/n.

  (ii) enforcing that the denominator is an unsigned type doesn't actually make
       anything simpler internally because you still have to check that the
       denominator is > 0, and now you have to deal with the hassle of
       considering _two_ internal integer types with all the consequent issues
       of conversion, arithmetic etc.


 Note that english wikipedia says that denumerator is integer not equal to zero:
 http://en.wikipedia.org/wiki/Rational_number
 It's interesting: have we got 2 different mathematics, or it's just mistake in
 wikipedia? I didn't read any english mathematics books, but the GMP library
 agree with me:
 "All rational arithmetic functions assume operands have a canonical form, and
 canonicalize their result. The canonical from means that the denominator and
the
 numerator have no common factors, and that the denominator is positive. Zero
has
 the unique representation 0/1."
 http://gmplib.org/manual/Rational-Number-Functions.html#Rational-Number-Functions

 May be you are rigth and store numerator and denumerator type is too complex,
 but it have some benefits. For example, we can save some memory with BigUint
 type because it doesn't store unnecessary sign.


We don't have any mathematical disagreement, it's merely that using two 
different types for numerator and denominator leads to unnecessary complexity 
for no meaningful gain.

Re: BigInt -- how much memory do you think you're saving by eliminating 
signed-ness, compared to the typical anticipated size of a BigInt?  BigInt 
itself is just a BigUInt plus a bool :-)


 I don't know, I just found it in source code:
 https://github.com/D-Programming-Language/phobos/blob/master/std/bigint.d


It's because to implement a big integer you need to store 2 things:

    (i) the magnitude (absolute value) of the number, which is what BigUInt is
        there for;

   (ii) the sign of the number, which is just a boolean true/false == +/-.

Because the type has (theoretically) no limits to its range, there's no point
in 
making public the "unsigned" type, because you don't gain anything by this. 
For 
built-in integer types it matters because each unique value in the memory 
corresponds to a unique integer value, so you have constrained range and having 
an unsigned type doubles the upper range of the type.

Compare to floating-point numbers where again the signed-ness is a single bit 
and so there's no gain to having an unsigned floating-point type.
Oct 01 2013
prev sibling next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Oct 01, 2013 at 08:26:49AM +0200, ilya-stromberg wrote:

 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:


[...]

This is needlessly complex. We should just use signed types for both
numerator and denominator, and handle sign by a canonicalization
operation (canonicalization is already needed for reducing the fraction
to its lowest terms anyway, and if gcd is implemented appropriately, it
will return a number of appropriate sign to cancel out the negative sign
in the denominator (or numerator), so this doesn't even need to be a
separate check).


T

-- 
A computer doesn't mind if its programs are put to purposes that don't match
their names. -- D. Knuth
Oct 01 2013
prev sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 01/10/13 16:49, H. S. Teoh wrote:

 This is needlessly complex. We should just use signed types for both
 numerator and denominator, and handle sign by a canonicalization
 operation (canonicalization is already needed for reducing the fraction
 to its lowest terms anyway, and if gcd is implemented appropriately, it
 will return a number of appropriate sign to cancel out the negative sign
 in the denominator (or numerator), so this doesn't even need to be a
 separate check).


That's how it's done.  But your remark made me realize that correction of the 
signs is not checked in the unittests, so I've added a couple such checks. :-)
Oct 02 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 28/09/13 13:15, Joseph Rushton Wakeling wrote:

 Well, obviously.  Actually I'm not unhappy with David's name per se but with
the
 idea of being inconsistent with how other libraries name these methods.
 Boost.Rational uses the full names, I'll have a look around and see how other
 libraries name them and whether there is consensus.


Boost.Rational, Python's numbers.Rational, Ruby's Rational, Haskell's Ratio,
and 
both Common Lisp and Scheme's in-built rational number support all use
numerator 
and denominator for their corresponding method names:
http://www.boost.org/doc/libs/1_54_0/libs/rational/rational.html#Numerator%20and%20Denominator
http://docs.python.org/2/library/numbers.html#numbers.Rational
http://docs.python.org/3/library/numbers.html#numbers.Rational
http://www.ruby-doc.org/core-2.0.0/Rational.html
http://www.haskell.org/onlinereport/ratio.html
http://www.lispworks.com/documentation/lw50/CLHS/Body/f_numera.htm
http://www.franz.com/support/documentation/current/ansicl/dictentr/numerato.htm
http://www.r6rs.org/final/html/r6rs/r6rs-Z-H-2.html#node_toc_node_sec_11.7.1

I also came across a Rational.java here which follows the same convention, but
I 
don't know how "official" this library is:
http://introcs.cs.princeton.edu/java/32class/Rational.java.html



letter:-)
http://exif-utils.googlecode.com/svn/trunk/ExifUtils/ExifUtils/Rational.cs

The one standout I came across was Julia, which uses "num" and "den":
http://docs.julialang.org/en/latest/manual/complex-and-rational-numbers/#rational-numbers

... and also implements an operator // to indicate rational division, which 
looks nice at first glance and then had me thinking, "Oh no, many bugs will
come 
from this ..."

In summary, I think we have other-language consensus here, and I'm going to
make 
the switch to "numerator", "denominator" as the public method names. 
Objections 
can always be brought up at review time :-)
Oct 01 2013
prev sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 01/10/13 11:07, Joseph Rushton Wakeling wrote:

 In summary, I think we have other-language consensus here, and I'm going to
make
 the switch to "numerator", "denominator" as the public method names. 
Objections
 can always be brought up at review time :-)


https://github.com/WebDrake/Rational/issues/9

... fixed by:
https://github.com/WebDrake/Rational/commit/69efbaee6bfec9b3cad4192b159cfbbb1dd70711
Oct 01 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 9/27/2013 6:47 AM, Joseph Rushton Wakeling wrote:

 [...]


Sounds like we have a new champion! Thanks, Joseph!
Sep 27 2013
next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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 feel more like a cheerleader than a champion -- David did all the hard work!
;-)
Sep 28 2013
next sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
28-Sep-2013 15:11, Joseph Rushton Wakeling пишет:

 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 feel more like a cheerleader than a champion -- David did all the hard
 work! ;-)


Which is a good thing. There is some much of hard work done on D 
libraries that is just piled up in the dusty corner, including Phobos 
candidates.

-- 
Dmitry Olshansky
Sep 28 2013
parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Saturday, 28 September 2013 at 11:29:25 UTC, Dmitry Olshansky 
wrote:

 28-Sep-2013 15:11, Joseph Rushton Wakeling пишет:

 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 feel more like a cheerleader than a champion -- David did 
 all the hard
 work! ;-)


 Which is a good thing. There is some much of hard work done on 
 D libraries that is just piled up in the dusty corner, 
 including Phobos candidates.


There is a lot of this. Many of them are recoverable: it never 
seems to take me long to grok someone work in D compared to many 
other languages, overall the abstractions tend to be natural and 
intuitive.
Sep 29 2013
prev sibling next sibling parent reply "Joseph Rushton Wakeling" <joseph.wakeling webdrake.net> writes:
On Saturday, 28 September 2013 at 11:11:33 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 feel more like a cheerleader than a champion -- David did all 
 the hard work! ;-)


Slightly bad news -- today I slipped and fell quite heavily on my 
right shoulder.  The hospital tells me there's nothing wrong that 
a couple of days' ice + painkillers won't fix, but proper 
two-handed typing is a bit painful in the meantime, so it'll 
probably be a few days before I get onto this.

I wish I could say that I fell while doing cheerleading 
somersaults, but alas it was nothing so cool -- a wet 
paving-stone on a rainy day ... :-P
Sep 29 2013
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/29/13 11:44 AM, Joseph Rushton Wakeling wrote:

 On Saturday, 28 September 2013 at 11:11:33 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 feel more like a cheerleader than a champion -- David did all the
 hard work! ;-)


 Slightly bad news -- today I slipped and fell quite heavily on my right
 shoulder.  The hospital tells me there's nothing wrong that a couple of
 days' ice + painkillers won't fix, but proper two-handed typing is a bit
 painful in the meantime, so it'll probably be a few days before I get
 onto this.

 I wish I could say that I fell while doing cheerleading somersaults, but
 alas it was nothing so cool -- a wet paving-stone on a rainy day ... :-P


Get better soon!

Andrei
Sep 29 2013
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 9/29/13, Joseph Rushton Wakeling <joseph.wakeling webdrake.net> wrote:

 Slightly bad news -- today I slipped and fell quite heavily on my
 right shoulder.  The hospital tells me there's nothing wrong that
 a couple of days' ice + painkillers won't fix, but proper
 two-handed typing is a bit painful in the meantime, so it'll
 probably be a few days before I get onto this.

 I wish I could say that I fell while doing cheerleading
 somersaults, but alas it was nothing so cool -- a wet
 paving-stone on a rainy day ... :-P


I only find this amusing because I went out cycling today, it started
to rain heavily, and then I fell on my *left* shoulder after crossing
the street. First time I've fallen from a bike in 15 years. I really
don't know why that white paint that marks the crossing part of the
street is so slippery when wet.
Sep 29 2013
prev sibling next sibling parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Sunday, 29 September 2013 at 18:44:08 UTC, Joseph Rushton 
Wakeling wrote:

 On Saturday, 28 September 2013 at 11:11:33 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 feel more like a cheerleader than a champion -- David did 
 all the hard work! ;-)


 Slightly bad news -- today I slipped and fell quite heavily on 
 my right shoulder.  The hospital tells me there's nothing wrong 
 that a couple of days' ice + painkillers won't fix, but proper 
 two-handed typing is a bit painful in the meantime, so it'll 
 probably be a few days before I get onto this.

 I wish I could say that I fell while doing cheerleading 
 somersaults, but alas it was nothing so cool -- a wet 
 paving-stone on a rainy day ... :-P


That sucks, hope it get better quick for you. Hey, at least you 
weren't this guy: http://www.livememe.com/nwndqkh.jpg
Sep 29 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 29/09/13 21:01, Andrej Mitrovic wrote:

 I only find this amusing because I went out cycling today, it started
 to rain heavily, and then I fell on my *left* shoulder after crossing
 the street. First time I've fallen from a bike in 15 years. I really
 don't know why that white paint that marks the crossing part of the
 street is so slippery when wet.


Ack, sympathies!

Hey, the Rust people have a new release out -- does it include a weather
control 
function?  <programming language conspiracy theory ... />

I managed to find a comfortable typing position propped up on the sofa with a 
couple of pillows, so hopefully I'll be more productive than planned.  Anyway, 
best wishes to you too for a speedy recovery!
Sep 30 2013
prev sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
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
Sep 30 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 9/28/2013 4:11 AM, 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 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.
Sep 29 2013
parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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? :-)
Sep 30 2013
prev sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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?
Sep 30 2013
parent reply "MrSmith" <mrsmith33 yandex.ru> writes:
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
Sep 30 2013
parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
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?
Sep 30 2013
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Fri, Sep 27, 2013 at 03:47:20PM +0200, Joseph Rushton Wakeling wrote:
[...]

 I've made a fork <https://github.com/WebDrake/Rational> and tweaked
 the code to be in line with current D/Phobos style (David Simcha's
 last update is from over 2 years ago so it's out of date in that
 respect).  I did this manually, line by line, so I could have a
 proper read-through of the code.
 
 On the basis of that read-through it looks pretty comprehensive and
 well-thought-out.  There are various workarounds for bugs which
 probably need to be re-examined depending on whether those bugs have
 been fixed or not.  In any case it compiles fine with current DMD,
 and the unittests are thorough and have 94% code coverage -- good
 job, David!
 
 One thing I don't like: David uses "num" and "denom" for the public
 methods to get numerator and denominator.


For the record, I'm OK with it. I don't like "numerator" and
"denominator" as it's needlessly verbose. It's already clear what "num"
and "denom" mean in a rational number library, no need to spell it all
out.

But, others may have a different opinion on this. As far as the review
is concerned, I don't mind either way.


[...]

 Anyway, if anyone else wants to take a look through and give me any
 comments, they'd be welcome.


I'll see if I can take a look at it sometime -- no guarantees, though,
as I'm busy with other things these days.



 (Bearophile, I will add your requested Fraction, but I'd like to get
 more info on what everyone else thinks of the code as it is before
 making any additions:-)


[...]

Honestly, I don't think that's necessary. It's just a one-line alias,
trivial to add in user code. For Phobos code, I think writing
Rational!BigInt is better, to make it clear what underlying number type
is being used to instantiate the template.


T

-- 
One Word to write them all, One Access to find them, One Excel to count them
all, And thus to Windows bind them. -- Mike Champion
Sep 27 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 9/26/13 7:00 AM, Joseph Rushton Wakeling wrote:

 On 25/09/13 19:21, ponce wrote:

 On Wednesday, 25 September 2013 at 17:15:25 UTC, ponce wrote:

 On Wednesday, 25 September 2013 at 16:59:15 UTC, H. S. Teoh wrote:

 Does anybody happen to have a working rational number library in D?




 Also David Simcha wrote a better one:
 https://github.com/dsimcha/Rational/blob/master/rational.d


 Why was this never incorporated into Phobos?


Probably because it wasn't proposed for review. I'm favorable to 
integrating it.

Andrei
Sep 26 2013
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Sep 26, 2013 at 04:50:47PM -0700, Andrei Alexandrescu wrote:

 On 9/26/13 7:00 AM, Joseph Rushton Wakeling wrote:

On 25/09/13 19:21, ponce wrote:

On Wednesday, 25 September 2013 at 17:15:25 UTC, ponce wrote:

On Wednesday, 25 September 2013 at 16:59:15 UTC, H. S. Teoh wrote:

Does anybody happen to have a working rational number library in
D?




Also David Simcha wrote a better one:
https://github.com/dsimcha/Rational/blob/master/rational.d


Why was this never incorporated into Phobos?


 
 Probably because it wasn't proposed for review. I'm favorable to
 integrating it.


[...]

Me too.


T

-- 
2+2=4. 2*2=4. 2^2=4. Therefore, +, *, and ^ are the same operation.
Sep 26 2013