Re: gamma() and lgamma() functions

From: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: gamma() and lgamma() functions
Date: 2025-03-25 05:01:19
Message-ID: CAK98qZ1g2YPHPbioD4PgHWe5Jgs7JuUEBMEmYbB5_1fgZ2X7_g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Dean,

Thanks for the patch!

I reviewed the patch and it looks good to me. I’ve run some tests, and
everything works as expected.

I have one minor thought:

On Sun, Mar 2, 2025 at 2:30 AM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
wrote:

> On Thu, 14 Nov 2024 at 22:35, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
> wrote:
> >
> > On Thu, 14 Nov 2024 at 16:28, Dmitry Koval <d(dot)koval(at)postgrespro(dot)ru>
> wrote:
> > >
> > > >SELECT gamma(float8 '1e-320');
> > > ERROR: value out of range: overflow
> > >
> > > >SELECT gamma(float8 '0');
> > > gamma
> > > ----------
> > > Infinity
> > > (1 row)
> > >
> > > Perhaps it would be logical if the behavior in these cases was the same
> > > (either ERROR or 'Infinity')?
> >
> > In general, I think that having gamma() throw overflow errors is
> > useful for spotting real problems in user code.
> >
>
> Thinking about this afresh, I remain of the opinion that having the
> gamma function throw overflow errors is useful for inputs that are too
> large, like gamma(200). But then, if we're going to do that, maybe we
> should just do the same for other invalid inputs (zero, negative
> integers, and -Inf). That resolves the consistency issue with inputs
> very close to zero, and seems like a practical solution.
>

Raising an error instead of silently returning Inf for invalid inputs
like zero, negative integers, and -Inf makes sense to me.

I also wondered whether we should distinguish domain errors
(e.g., zero or negative integers) from range errors (e.g., overflow).
I tested tgamma() and lgamma() on Ubuntu 24.04, macOS 15.3.2, and
Windows 11 (code attached).

Here’s a summary of return values, errnos, and exceptions on these
platforms: (errno=33 is EDOM and errno=34 is ERANGE)

tgamma(0):
Ubuntu : inf, errno=34, FE_DIVBYZERO
macOS : inf, errno=0, FE_DIVBYZERO
Windows: inf, errno=34, FE_DIVBYZERO

lgamma(0):
Ubuntu : inf, errno=34, FE_DIVBYZERO
macOS : inf, errno=0
Windows: inf, errno=34, FE_DIVBYZERO

tgamma(-1):
Ubuntu : nan, errno=33, FE_INVALID
macOS : nan, errno=0, FE_INVALID
Windows: nan, errno=33, FE_INVALID

lgamma(-1):
Ubuntu : inf, errno=34, FE_DIVBYZERO
macOS : inf, errno=0, FE_DIVBYZERO
Windows: inf, errno=34, FE_DIVBYZERO

tgamma(-1000.5):
Ubuntu : -0.0, errno=34, FE_UNDERFLOW
macOS : -0.0, errno=0
Windows: 0.0, errno=34

lgamma(-1000.5):
Ubuntu : -5914.44, errno=0
macOS : -5914.44, errno=0
Windows: -5914.44, errno=0

tgamma(1000):
Ubuntu : inf, errno=34, FE_OVERFLOW
macOS : inf, errno=0, FE_OVERFLOW
Windows: inf, errno=34, FE_OVERFLOW

lgamma(1000):
Ubuntu : 5905.22, errno=0
macOS : 5905.22, errno=0
Windows: 5905.22, errno=0

tgamma(+inf):
Ubuntu : inf, errno=0
macOS : inf, errno=0
Windows: inf, errno=0

lgamma(+inf):
Ubuntu : inf, errno=0
macOS : inf, errno=0
Windows: inf, errno=0

tgamma(-inf):
Ubuntu : nan, errno=33, FE_INVALID
macOS : nan, errno=0, FE_INVALID
Windows: nan, errno=33, FE_INVALID

lgamma(-inf):
Ubuntu : inf, errno=0
macOS : inf, errno=0
Windows: inf, errno=0

In the current implementation, float_overflow_error() is used for both
domain and range (overflow) errors. If we did want to distinguish
them, maybe we could use float_zero_divide_error() for domain errors?
Not sure it adds much value, though - just a thought, and I’m fine
either way.

OTOH I wondered if we could leverage fetestexcept(), but then I saw
this comment in dcos():

* check for errors. POSIX allows an error to be reported either via
> * errno or via fetestexcept(), but currently we only support checking
> * errno. (fetestexcept() is rumored to report underflow unreasonably
> * early on some platforms, so it's not clear that believing it would be a
> * net improvement anyway.)

So again, I’m totally fine with keeping the current semantics as-is.
That said, it might be helpful to update the comment in dlgamma():
+ /*
+ * If an ERANGE error occurs, it means there is an overflow.
+ *
to clarify that an ERANGE error can also indicate a pole error (e.g.,
input 0 or a negative integer), not just overflow.

Thanks,
Alex

Attachment Content-Type Size
gamma_test.no-cfbot application/octet-stream 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shubham Khanna 2025-03-25 05:06:00 Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.
Previous Message Tom Lane 2025-03-25 04:32:05 Re: query_id: jumble names of temp tables for better pg_stat_statement UX