fix: lgamma handles negative reals correctly, respects config.predictable

[ShehabSherif0]

Problem

lgamma does not handle negative non-integer real inputs correctly:

1. lgamma(math.complex(-1.5, 0)) returns Complex(NaN, 0) instead of the well-defined LogGamma value
2. lgamma(-1.5) (plain number) returns NaN even when config.predictable is false, which is inconsistent with how sqrt, log, pow, asin, etc. handle real inputs that produce complex results

Expected behavior (from Wolfram Alpha LogGamma[z]):

math.lgamma(-1.5)                    // Complex(0.860..., -6.283...)
math.lgamma(math.complex(-1.5, 0))   // Complex(0.860..., -6.283...)

Solution

Two changes, following the established config.predictable pattern used by sqrt, log, log2, log10, log1p, pow, asin, etc.:

1. Number handler respects config.predictable

Added config as a factory dependency. The number handler now checks:

• x >= 0 or config.predictable === true: return lgammaNumber(x) (number result, NaN for negatives)
• config.predictable === false (default) and x < 0: route through lgammaComplex(new Complex(x, 0)) to return the proper Complex result

2. Complex handler no longer short-circuits negative reals

- } else if (n.im === 0) {
+ } else if (n.im === 0 && n.re >= 0) {

Negative reals now fall through to the reflection formula path, which already handles them correctly.

Documentation

Updated the lgamma docstring to explain:

• It computes the principal branch of the LogGamma special function (the analytic continuation of ln(Gamma(z)) for positive reals), not just log(gamma(z))
• The config.predictable behavior for negative real inputs
• Correct examples showing Complex output for negative inputs

Testing

• Negative non-integer reals now expect Complex results (verified against Wolfram Alpha reference values)
• Added predictable: true test confirming NaN return for negative number inputs
• Added Complex(-x, 0) tests verifying consistency between number and Complex input paths
• All 6644 tests pass (11 lgamma-specific tests, 0 failures)

Fixes #3604

[Copilot]

Pull request overview

Fixes lgamma(Complex(x, 0)) returning Complex(NaN, 0) for negative real x by preventing an overly-broad shortcut to the real-number implementation, and adds regression coverage plus clarifying inline documentation for complex branch behavior.

Changes:

• Restrict the lgammaNumber shortcut in lgammaComplex to non-negative real complex inputs (im === 0 && re >= 0).
• Add unit tests covering negative real complex inputs with zero imaginary part (reflection-formula path).
• Expand the JSDoc description/examples to clarify principal-branch behavior and the real-vs-complex negative-input distinction.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/function/probability/lgamma.js	Adjusts complex dispatch logic and clarifies documentation around branch behavior and negative real handling.
test/unit-tests/function/probability/lgamma.test.js	Adds regression tests for lgamma(Complex(negative, 0)) with reference values.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

PR description says this fixes issue #3604, but number inputs are still handled by lgammaNumber, and this file’s documentation/tests indicate lgamma(<negative real number>) is expected to return NaN. If #3604 is about negative number inputs, this PR won’t resolve it—consider adjusting the issue linkage/wording (or changing lgammaNumber) to avoid implying the issue is fully fixed.

[ShehabSherif0]

Good observation. Issue #3604 covers two aspects:

1. lgamma(Complex(-1.5, 0)) returning Complex(NaN, 0) -- this is the code bug this PR fixes.
2. lgamma(-1.5) (plain number) returning NaN -- this is expected and intentional, because the result is complex and a real-number function cannot return a complex value.

The maintainer discussion on #3604 converged on this exact approach. @gwhitney wrote:

"Given that lgammaNumber is an internal helper, I am not too concerned with the details of its behavior as long as the functions in the public API are operating correctly."

And outlined two action items:

1. Fix the bug returning NaNs for lgamma(z) when z is a negative non-integer real as a Complex -- done in this PR.
2. Properly document lgamma's behavior -- done in this PR (the docstring now explains that real inputs return NaN for negatives, and users should pass a Complex input for the full result).

So the issue linkage is accurate; both parts of the agreed resolution are addressed here.

[gwhitney]

2. lgamma(-1.5) (plain number) returning NaN -- this is expected and intentional, because the result is complex and a real-number function cannot return a complex value.

First of all, thanks for the help in trying to resolve this, but please use caution in using AI agents to resolve issues and make pull requests -- they often introduce duplicative or otherwise hard-to-main code.

That said, the quoted point (2) is not a correct characterization of how mathjs handles functions of real (plain JavaScript) numbers that may be interpreted to return Complex values for some (but not all) inputs. Please see the final comment from #3604 for a more detailed description of the desired behavior of lgamma on real inputs, and adjust this PR accordingly. In particular, note the dependence of the behavior of lgamma() on negative non-integer inputs on the config.predictable setting. (Also please note this is not the full review, it's just a key first comment to get it to a point where performing a full review is worthwhile; thanks for understanding.)

[ShehabSherif0]

Thanks for the correction, and for the patience. You were correct about this. I went back and read your final comment on #3604 carefully and realized I had completely missed the config.predictable requirement.

I've now reworked the PR to follow the same pattern that sqrt, log, log2, log10, pow, asin, and the others use for this exact situation. Specifically:

• Added config as a factory dependency for lgamma
• The number handler now checks: if the input is non-negative or config.predictable is true, it goes through lgammaNumber (returning NaN for negatives in predictable mode, preserving the number return type guarantee). Otherwise it routes through lgammaComplex(new Complex(x, 0)) to produce the correct principal branch result.
• The complex handler fix from before (restricting the lgammaNumber shortcut to n.im === 0 && n.re >= 0) is still in place.

So now:

// Default (config.predictable = false):
math.lgamma(-1.5)                    // Complex(0.860..., -6.283...)
math.lgamma(math.complex(-1.5, 0))   // Complex(0.860..., -6.283...)  (consistent)

// With config.predictable = true:
math.lgamma(-1.5)                    // NaN

I've also updated the docstring to properly describe lgamma as the principal branch of the LogGamma special function (not just log(gamma(z))), and to document the config.predictable behavior.

Tests are updated with Wolfram Alpha reference values for the negative non-integer cases, plus a dedicated predictable: true test. All 6644 tests in the suite pass.

Point taken on the AI tooling as well. I'll be more careful to verify the library's conventions before jumping in.

[gwhitney]

Apologies the maintainers are a bit swamped right now; I will get to this review as soon as I can. Thanks for understanding.