Skip to content

Fix weighted inverse covariance calculation in sWeights#205

Open
TingzhenXiao wants to merge 5 commits into
scikit-hep:mainfrom
TingzhenXiao:patch-1
Open

Fix weighted inverse covariance calculation in sWeights#205
TingzhenXiao wants to merge 5 commits into
scikit-hep:mainfrom
TingzhenXiao:patch-1

Conversation

@TingzhenXiao

Copy link
Copy Markdown

This PR updates the computation of the inverse covariance matrix in compute_sweights when sample_weight is provided.

Previously, the matrix was evaluated as an unweighted event sum. For efficiency-weighted samples, this does not correspond to the desired weighted approximation of the integral entering the sWeight covariance matrix.

The change replaces the unweighted sum by a weighted matrix product,

Vinv = pN.T @ (sample_weight[:, None] * pN)

where pN contains the component PDFs divided by the total extended model PDF. This makes the numerical evaluation consistent with the weighted sample used in the fit.

The default behaviour is unchanged when sample_weight=None.

@codecov-commenter

codecov-commenter commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.71%. Comparing base (4e685ed) to head (daf3196).

Files with missing lines Patch % Lines
src/hepstats/splot/sweights.py 0.00% 12 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #205      +/-   ##
========================================
- Coverage   3.74%   3.71%   -0.03%     
========================================
  Files         26      26              
  Lines       1282    1291       +9     
========================================
  Hits          48      48              
- Misses      1234    1243       +9     
Flag Coverage Δ
unittests 3.71% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TingzhenXiao

Copy link
Copy Markdown
Author

Hi @eduardo-rodrigues,

My supervisor suggested that I tag you here, since the original sWeights implementation was written by you.

This PR adds optional sample_weight support to compute_sweights. The motivation is that, for efficiency-weighted samples, the inverse covariance matrix should be evaluated with the same event weights used in the fit, namely

Vinv = pN.T @ (sample_weight[:, None] * pN)

while keeping the default behaviour unchanged when sample_weight=None.

Codecov is currently reporting missing coverage for the newly added sample_weight branch. I can add a small unit test covering:

  1. the default sample_weight=None behaviour,
  2. a non-trivial sample_weight array,
  3. invalid shape/length checks.

Could you please let me know whether this API/design looks reasonable, or whether you would prefer a different implementation?

@eduardo-rodrigues eduardo-rodrigues added the enhancement New feature or request label Jun 19, 2026
Comment thread src/hepstats/splot/sweights.py
Comment thread src/hepstats/splot/sweights.py Outdated

@eduardo-rodrigues eduardo-rodrigues left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dear @TingzhenXiao, thank you very much for the interest in the package and for getting in touch. Are you an LHCb colleague, by chance?

I was not the original author, I think. It must have been @marinang. And then @jonas-eschle took over the maintenance and developments. Let me cc/ping @jonas-eschle as his review and opinions would be most welcome.

Thank you again.

@eduardo-rodrigues

Copy link
Copy Markdown
Member

(While at it you can,I reckon, remove the CI on macos-15, which is deprecated/retired.)

@TingzhenXiao

Copy link
Copy Markdown
Author

Dear @TingzhenXiao, thank you very much for the interest in the package and for getting in touch. Are you an LHCb colleague, by chance?

I was not the original author, I think. It must have been @marinang. And then @jonas-eschle took over the maintenance and developments. Let me cc/ping @jonas-eschle as his review and opinions would be most welcome.

Thank you again.

Yes, I am currently working on an LHCb-related project with Prof. Eluned Smith at MIT

Improve sample_weight handling in sWeights
@TingzhenXiao

Copy link
Copy Markdown
Author

Thanks, I have updated the implementation to keep the original unweighted formula for the default sample_weight=None case and added the corresponding docstring.

@eduardo-rodrigues

Copy link
Copy Markdown
Member

Thanks for the updates so far. Ping again when done with the rest :-).

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants