Skip to content

BEHAVIOR: use subtraction in UnbinnedNLL - #574

Merged
redeboer merged 11 commits into
mainfrom
ENH/nll-computation
Aug 5, 2026
Merged

BEHAVIOR: use subtraction in UnbinnedNLL#574
redeboer merged 11 commits into
mainfrom
ENH/nll-computation

Conversation

@redeboer

@redeboer redeboer commented Aug 5, 2026

Copy link
Copy Markdown
Member

Closes #565

⚙️ Enhancements

  • UnbinnedNLL.__call__ now computes N * log(integral) - sum(log(data)) instead of -sum(log(data / integral)), which is the formula its own docstring already documents. This drops the full normalized-likelihood temporary: roughly 1.5x faster at formula level on NumPy, and ~1.15x on the full estimator call, where the intensity evaluation dominates.

❗ Behavioral changes

  • The two formulations are only equivalent when the normalization integral and all data intensities are strictly positive. Where the division form stayed finite for a positive ratio of two negative numbers, the subtraction form takes the two logarithms separately and returns NaN.
  • Concretely: a model with unconstrained signed coefficients (a very common way to write a mixture) can be pushed into negative intensities by an unbounded optimizer, and the fit now returns NaN instead of converging on a physically meaningless minimum. This is not hypothetical — it broke test_optimize_all_parameters for all four back-ends under ScipyMinimizer.
  • If you hit this, make the model non-negative by construction rather than relying on the old behavior. Squaring the coefficients is enough, and is what the test and benchmark models now do:
    expression = a**2 * gaussian(x, mu1, sigma1) + b**2 * gaussian(x, mu2, sigma2)
    parameter_defaults = {a: np.sqrt(0.15), b: np.sqrt(0.05)}
    Fitted values are then square roots of the old ones, so any hard-coded reference values need regenerating.

🖱️ Developer experience

  • New benchmarks/unbinned_nll.py compares the two formulations over NumPy, Numba, JAX, and TensorFlow. Each back-end is timed with its own warm-up and synchronisation (block_until_ready() for JAX, .numpy() for TensorFlow, a compilation call for Numba), so that the numbers are actually comparable, and every benchmark asserts its result against a NumPy reference.
  • The same file also benchmarks the full UnbinnedNLL estimator per back-end, so that the formula-level difference can be weighed against the intensity evaluation that dominates a real fit.
  • The Numba implementations are written as explicit prange loops under njit(parallel=True) instead of as array expressions, since that is where Numba is competitive at all.
  • poe benchmark now takes optional positional paths, so that a single file can be run in isolation instead of the whole benchmarks/ directory:
    poe benchmark benchmarks/unbinned_nll.py
  • benchmarks/expression.py and the fit test model square their mixture coefficients, so that they are non-negative by construction.

🔨 Maintenance

  • Renamed the private back-end attributes __mean_function, __sum_function, and __log_function to __mean, __sum, and __log, and the local bare_intensities to data_intensities.

Squash commit messages

* DX: allow passing paths to `poe benchmark`
* DX: benchmark `UnbinnedNLL` implementations per back-end
* DX: enforce positive intensities in test and benchmark models
* MAINT: rename private back-end attributes in `UnbinnedNLL`

@redeboer redeboer added this to the 0.4.17 milestone Aug 5, 2026
@redeboer redeboer self-assigned this Aug 5, 2026
@redeboer redeboer added the ⚙️ Enhancement Improvements and optimizations of existing features label Aug 5, 2026
redeboer

This comment was marked as resolved.

@redeboer redeboer changed the title ENH: speed up UnbinnedNLL implementation for NumPy DX: benchmark UnbinnedNLL implementations Aug 5, 2026
@redeboer redeboer added 🔨 Maintenance Maintenance and upkeep improvements 🖱️ DX Improvements to the Developer Experience and removed ⚙️ Enhancement Improvements and optimizations of existing features labels Aug 5, 2026
@redeboer redeboer changed the title DX: benchmark UnbinnedNLL implementations ENH: speed up UnbinnedNLL implementation Aug 5, 2026
@redeboer redeboer added the ⚙️ Enhancement Improvements and optimizations of existing features label Aug 5, 2026
@redeboer redeboer changed the title ENH: speed up UnbinnedNLL implementation BEHAVIOR: use subtraction in UnbinnedNLL Aug 5, 2026
@redeboer redeboer added the ❗ Behavior Changes that may affect the framework output label Aug 5, 2026
@redeboer
redeboer force-pushed the ENH/nll-computation branch from a83c1b6 to 983ecd0 Compare August 5, 2026 19:45
@redeboer redeboer removed the 🔨 Maintenance Maintenance and upkeep improvements label Aug 5, 2026
@redeboer
redeboer merged commit 37c1c42 into main Aug 5, 2026
27 checks passed
@redeboer
redeboer deleted the ENH/nll-computation branch August 5, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

❗ Behavior Changes that may affect the framework output 🖱️ DX Improvements to the Developer Experience ⚙️ Enhancement Improvements and optimizations of existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compute UnbinnedNLL with substraction rather than division

1 participant