@larsmans the `lgamma` you mention first is from `amp_math` which is a
multiprocessing library, I'm not sure it's the right thing (and also how do
we conditionally import from a different library only on windows?). Also
would compiling with `cython --cplus` cause other differences?

I really like John D. Cook's version and I'm +1 for including it instead. I
didn't find it the first time, otherwise I would definitely bundle it
instead of this current mess.


On Sun, Jan 13, 2013 at 10:04 AM, Lars Buitinck <notifications@github.com>wrote:

> In sklearn/utils/src/e_lgamma_r.h:
>
> > + *	      computation of sin(pi*(-x)).
> > + *		
> > + *   5. Special Cases
> > + *		lgamma(2+s) ~ s*(1-Euler) for tiny s
> > + *		lgamma(1)=lgamma(2)=0
> > + *		lgamma(x) ~ -log(x) for tiny x
> > + *		lgamma(0) = lgamma(inf) = inf
> > + *	 	lgamma(-integer) = +-inf
> > + *	
> > + */
> > +
> > +#include <math.h>
> > +#include <stdio.h>
> > +
> > +int signgam = 0;
> > +
>
> I'm sorry, but I'm -1 on including this code; any identifier with __ is
> reserved for the implementation and casting a double* to int* is instant
> undefined behavior, even apart from the width issue. I just fixed an issue
> with a similar identifier that caused compilation on Cygwin and Solaris to
> fail.
>
> If we want to lgamma on Windows, then I suggest we either compile the
> relevant file with cython --cplus, because then lgamma is available<http://msdn.microsoft.com/en-us/library/hh308219.aspx>(after fixing some imports, I guess), or we use John
> Cook's version <http://www.johndcook.com/cpp_gamma.html>, which is in the public
> domain <http://www.johndcook.com/stand_alone_code.html> and much more
> readable than this.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/scikit-learn/scikit-learn/pull/1529/files#r2628788>.
>
>