Skip to content

lib: two degenerate-input contracts return a plausible-but-wrong number (factorial of a negative; time_of_flight's "positive root") #641

Description

@InauguralPhysicist

Two minor domain-boundary cases where a function returns a confident number for input it can't actually answer. Grouped: same class, same fix shape, one PR.


1. factorial of -3 returns 1

lib/probability.eigs:

load_file of "lib/probability.eigs"
print of ["factorial of -3 =", factorial of -3]
["factorial of -3 =", 1]

n! is undefined for negative integers. The guard is n <= 1 -> 1, which is right for 0 and 1 and silently wrong below that. docs/STDLIB.md states factorial of n → n! with no domain, so a negative reaching it (from a subtraction, a loop index, user input) yields 1 rather than an error.

Knock-on: poisson_pmf accepts a negative k for the same reason — its factorial of k denominator quietly becomes 1, so it returns a plausible probability for an impossible event count.

Fix: if n < 0: raise (or return null — but decide and document it). Fix factorial and the shape is fixed for poisson_pmf too, but add an explicit k < 0 guard there so the diagnostic names the real problem.

Test: factorial of -3 raises; factorial of 0 == 1 and factorial of 1 == 1 still hold.


2. time_of_flight returns the trivial t=0 root when displacement is zero

lib/physics.eigs:56:

# time_of_flight — solve x = v₀·t + ½·a·t² for t (positive root)
print of ["time_of_flight of [20, -9.8, 0] =", time_of_flight of [20, -9.8, 0]]
["time_of_flight of [20, -9.8, 0] =", 0]

The roots for v0=20, a=-9.8, x=0 are t=0 and t=4.0816. The code returns the smallest non-negative root; its comment promises the positive root. 0 is not positive, and a positive root exists.

This one is a contract ambiguity, not a clear bug — I downgraded it from the reviewer's "major". Returning the first time the projectile reaches height x is the right answer for x > 0 (e.g. "when does it pass 10m going up?"), and that's what "smallest non-negative" gives. The two readings only diverge at x = 0, where the launch instant is technically a solution. projectile_time already exists for total flight duration, so time_of_flight isn't obliged to mean that.

So the defect is the comment, or the x=0 case, depending on the intended contract:

  • if it means "first time reaching x": the comment should say smallest non-negative root, and x=00 is correct as documented;
  • if it means "positive root" as written: the t1 >= 0 test should be t1 > 0 (with an epsilon), returning 4.0816.

Fix: decide which, then make the comment and the code agree. Whichever way, docs/STDLIB.md should state it — this is exactly the class where a physics user assumes the textbook meaning.

Test: pin whichever contract is chosen at x = 0, plus the x > 0 case that must keep returning the first crossing either way.


Found by Grok Build in a delegated read-only stdlib review; both reproduced and verified independently. Severity on #2 lowered from the reported "major" after checking the contract — the code is defensible, the comment isn't.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions