Skip to content

ENH: Add WeightedPoisson datafit with sample weights support - #341

Open
Vrinda12-tech wants to merge 2 commits into
scikit-learn-contrib:mainfrom
Vrinda12-tech:main
Open

ENH: Add WeightedPoisson datafit with sample weights support#341
Vrinda12-tech wants to merge 2 commits into
scikit-learn-contrib:mainfrom
Vrinda12-tech:main

Conversation

@Vrinda12-tech

Copy link
Copy Markdown

Summary

This PR adds a weighted variant of the Poisson datafit, enabling sample weights support for Poisson regression in skglm.

Implementation

  • New class WeightedPoisson in skglm/datafits/single_task.py
  • Follows the WeightedQuadratic pattern
  • Numba JIT compiled for performance
  • Sparse matrix support (CSC format)
  • Numerical stability: np.clip(Xw, -500, 500) prevents exp overflow
  • Safety floors in Lipschitz constants to prevent division by zero

Mathematical Derivation

The weighted Poisson loss is:

L = (1/Σw) * Σ( w_i * (exp((Xw)_i) - y_i * (Xw)_i) )

The weighted gradient:

∇L = (w * (exp(Xw) - y)) / Σw

The weighted Hessian:

H = (w * exp(Xw)) / Σw

Tests Added

  • ✅ Equality with unweighted Poisson (equal weights)
  • ✅ Correct scaling with arbitrary weights
  • ✅ Zero weight handling
  • ✅ Negative weight validation
  • ✅ Sparse matrix support
  • ✅ Lipschitz constant computation
  • ✅ Integration with GeneralizedLinearEstimator

All 7 tests are passing locally.

References

  • WeightedQuadratic implementation pattern
  • Poisson datafit for unweighted version

Copilot AI review requested due to automatic review settings June 30, 2026 08:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mathurinm mathurinm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for the initiative !
There are lots of tests, do you think you could shorten them a bit for easier maintenance ? like factorize some aprts

also, can you have a single test where you have weights equal to 0, 1 or 2, and test that you get the same result as fitting with duplicated and removed samples (weights=2 or 0 respectively). I think this would remove the need for testing the lipschitz constant for example

from .base import BaseDatafit, BaseMultitaskDatafit
from .single_task import (Quadratic, QuadraticSVC, Logistic, Huber, Poisson, Gamma,
Cox, WeightedQuadratic, QuadraticHessian,)
Cox, WeightedQuadratic, QuadraticHessian,WeightedPoisson)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need spaces after the commas

np.testing.assert_allclose(lasso.coef_, qpl1.coef_)
# check that it's not just because we got alpha too high and thus 0 coef
np.testing.assert_array_less(0.1, np.max(np.abs(qpl1.coef_)))
# =============================================================================

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like a duplicate of what's 3 lines below

Comment thread skglm/tests/test_datafits.py Outdated

def test_weighted_poisson_validation():
"""Test input validation for WeightedPoisson."""
import numpy as np

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can put the imports at the top of the file

@mathurinm

Copy link
Copy Markdown
Collaborator

@Badr-MOUFAD at some point I wonder if it wouldn't be easier if every datafit had weights by default... but not that much time to think about it

@Vrinda12-tech

Copy link
Copy Markdown
Author

@mathurinm Thank you for the detailed review. I have addressed all the points:

  1. Reduced the number of tests: I replaced test_weighted_poisson_equality, test_weighted_poisson_scaling, test_weighted_poisson_zero_weights, and test_weighted_poisson_lipschitz with a single duplication test (test_weighted_poisson_duplication).

  2. The duplication test: It sets weights to 0 (remove), 2 (duplicate), and 1 (keep), then compares the weighted model against an unweighted model fit on the manually duplicated/removed dataset. This implicitly validates the loss, gradient, Hessian, and Lipschitz constants.

  3. Alpha scaling: I initially tried to scale alpha for the unweighted model. I realized that because both losses are averaged (1/n for unweighted, 1/sum(w) for weighted), alpha should be identical for both models. The test now uses the same alpha and passes.

All 4 tests (duplication, validation, sparse, and integration) are passing locally.

Let me know if you have any other suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants