On Tue, 25 Mar 2025 at 05:01, Alexandra Wang <alexandra.wang....@gmail.com> wrote: > > I reviewed the patch and it looks good to me. I’ve run some tests, and > everything works as expected.
Thanks for the careful review and testing. > 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) Thanks for that. It's very useful to see how tgamma() and lgamma() behave on those different platforms. > 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. Hmm, I don't think "division by zero" would be the right error to use, if we did that. That's probably exposing some internal implementation detail, but I think it would look odd to users. Perhaps "input is out of range" would work, but I don't think it's really necessary to distinguish between these different types of errors. > 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. OK, thanks. I've updated that comment and committed it. Regards, Dean
