14 years, 11 months ago
(2010-02-16 15:58:21 UTC)
#3
http://codereview.appspot.com/207098/diff/1/3
File src/core/random-variable.h (right):
http://codereview.appspot.com/207098/diff/1/3#newcode417
src/core/random-variable.h:417: ParetoVariable (std::pair<double, double>
params, double b);
I understand why you have done this but it feels really really weird. I am not
sure that the following is much better but I would have probably tried to do
this instead to ensure backward compatibility and provide the needed
functionality. With that said, either approach works fine and is fine with me.
class ParetoVariableFromScaleAndShape : public ParetoVariable
{
// redefine constructor with 2 double arguments.
};
I agree. Tom suggested to use pair<>, and seems reasonable, as doing so we don't ...
14 years, 11 months ago
(2010-02-18 01:12:36 UTC)
#4
I agree. Tom suggested to use pair<>, and seems reasonable, as doing so we don't
need one more class just to add a different constructor.
Anyway, either solution works and it's fine for me, let's just decide which one
is to be implemented.
On 2010/02/18 01:12:36, Tommaso Pecorella wrote: > I agree. Tom suggested to use pair<>, and ...
14 years, 11 months ago
(2010-03-01 20:58:41 UTC)
#6
On 2010/02/18 01:12:36, Tommaso Pecorella wrote:
> I agree. Tom suggested to use pair<>, and seems reasonable, as doing so we
don't
> need one more class just to add a different constructor.
>
> Anyway, either solution works and it's fine for me, let's just decide which
one
> is to be implemented.
Tommaso,
It looks like you get to decide which solution to take. Are you happy with this
one? If so +1 for merge from me.
Aye, merging tonight Tommaso On 01/mar/2010, at 21.58, joshpelkey@gmail.com wrote: > On 2010/02/18 01:12:36, Tommaso ...
14 years, 11 months ago
(2010-03-01 23:46:24 UTC)
#7
Aye,
merging tonight
Tommaso
On 01/mar/2010, at 21.58, joshpelkey@gmail.com wrote:
> On 2010/02/18 01:12:36, Tommaso Pecorella wrote:
>> I agree. Tom suggested to use pair<>, and seems reasonable, as doing
> so we don't
>> need one more class just to add a different constructor.
>
>> Anyway, either solution works and it's fine for me, let's just decide
> which one
>> is to be implemented.
>
> Tommaso,
>
> It looks like you get to decide which solution to take. Are you happy
> with this one? If so +1 for merge from me.
>
> http://codereview.appspot.com/207098/show
Issue 207098: Pareto rng constructors using scale and shape instead of mean and shape
(Closed)
Created 14 years, 11 months ago by Tommaso Pecorella
Modified 14 years, 11 months ago
Reviewers: Mathieu Lacage, Josh Pelkey
Base URL:
Comments: 5