asymmetric traditional impacts; unblinding groups by lucalavezzo · Pull Request #133 · WMass/rabbit

lucalavezzo · 2026-04-30T13:15:54Z

No description provided.

…asym-impacts

…ssian constraints on params in ParamModels

davidwalter2

Here are a few comments to start with, please also add coverage for the new features in the CI, the asymmetric impacts but also maybe other things. If it's not too much work maybe break it up in separate PRs

davidwalter2 · 2026-06-11T17:08:11Z

+        nargs="+",
+        help="Regex(es) excluding nuisances from --asymImpacts.",
+    )
+    parser.add_argument(


I would remove that option and make it default to compute all nuisances with asymImpacts, then exclude the ones you like with --asymImpactsExclude or specify a list with the --asymImpactsInclude otherwise we have too many CLAs

davidwalter2 · 2026-06-11T17:52:22Z

+For each selected nuisance, find the asymmetric +/- 1 sigma points on the
+Delta(2NLL)=q likelihood contour via constrained minimization (contour_scan).
+The shifts of every fitter parameter at those points are the asymmetric
+impacts. Group impacts are obtained by quadrature envelope of the contained


I think the convention for grouped impacts in the case of traditional impacts is different, we should use the analogue to what is done in the gaussian case

davidwalter2 · 2026-06-11T17:55:01Z

+impacts. Group impacts are obtained by quadrature envelope of the contained
+nuisances, separately for the down and up sides.
+
+Nuisances that are structurally symmetric (logkhalfdiff identically zero)


I don't think this is true because nonlinear effects can lead to asymmetric constraints and impacts also for symmetric nuisances. I think by default we should do the most generic and compute for all nuisances and the user has to specify explicitly which he wants to include/exclude

davidwalter2 · 2026-06-11T17:57:56Z

+    q=1,
+    contour_xtol=1e-4,
+    contour_gtol=1e-4,
+    contour_maxiter=200,


I think the default should be set to effectively infinite, 5000 or so, since the user does not expect it to stop early

davidwalter2 · 2026-06-11T18:19:40Z



-def _gather_poi_noi_vector(v, noiidxs, nsignal_params=0):
+def _gather_poi_noi_vector(v, noiidxs, nsignal_params=0, nmodel_params=None):


Is this computing the impacts also for POUs? If so, why?

davidwalter2 · 2026-06-11T18:28:10Z

+        "uncertainty rows; their entries in the output are NaN.",
+    )
+    parser.add_argument(
+        "--paramModelPriors",


I think there should be no CLA for that, if the model defines priors they should be applied by default (since naively the user expects the priors to be applied in case he defines them). So either remove the option and leave it to the user to implement something in the model parser, or add the inverse of this option to disable model priors.

Per review on WMass#133 (parsing.py:252): there is no rabbit-side CLA anymore. If a ParamModel declares prior_sigmas, the priors are applied; whether and how to enable them (e.g. a token in the --paramModel spec) is the model's own decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on WMass#133 (asym_impacts.py:7): grouped asymmetric impacts now follow the traditional grouped-impact convention generalized to the asymmetric case. Per side, the group impact is sqrt(delta^T rho_GG^-1 delta) over the group'\''s scanned nuisances, with rho_GG the postfit correlation submatrix — the exact analogue of sqrt(v^T C_GG^-1 v) used for symmetric grouped impacts, reducing to it in the Gaussian limit and to a quadrature sum for uncorrelated nuisances. Verified on the test tensor that near-Gaussian groups reproduce the traditional grouped impacts to ~1e-3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Reconciles the review discussion on WMass#133 (rabbit_fit.py:603): when --externalPostfit provides a covariance it is used as before; when it does not (the fitresult was produced with --noHessian), the Hessian is recomputed at the loaded postfit point instead of silently writing no covariance. This enables the two-pass recipe for models whose full Hessian is infeasible during the fit: pass 1 fits with --noHessian, pass 2 reruns with --externalPostfit ... --noFit (without --noHessian) to obtain the covariance. Verified on the test tensor that the two-pass covariance is identical to the single-pass one, and that a fitresult with a covariance is still consumed without recomputation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on WMass#133: param_prior_sigmas / param_prior_means are stored once as TF constants holding the priors as declared (NaN where no prior); the compute-safe masked forms are derived inside _compute_lc, and the output metadata reads the arrays via .numpy(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

lucalavezzo · 2026-06-12T13:12:09Z

Thanks @davidwalter2 for the comments, I've addressed them and split up the PR into 3 more managable ones:

asym impacts #138
Unified constraint structure (cw/x0); ParamModel Gaussian priors #139
fit ops qol #140
Closing this one

Per review on #133: drop --asymImpactsAll and --asymImpactsMaxiter; --asymImpacts now scans every constrained nuisance by default (nonlinear effects can produce asymmetric impacts even for structurally symmetric templates) with --asymImpactsInclude/Exclude as the only gating. The trust-constr maxiter default is raised to 5000 so scans are not cut off early. The structural-symmetry skip remains available programmatically. Clarify in traditional_impacts that POUs only act as impact sources, never as impact rows. Add the global-asym-impacts test to the CI unit-test matrix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on WMass#133 (parsing.py:252): there is no rabbit-side CLA anymore. If a ParamModel declares prior_sigmas, the priors are applied; whether and how to enable them (e.g. a token in the --paramModel spec) is the model's own decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on WMass#133: param_prior_sigmas / param_prior_means are stored once as TF constants holding the priors as declared (NaN where no prior); the compute-safe masked forms are derived inside _compute_lc, and the output metadata reads the arrays via .numpy(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on #133: drop the unnecessary try/except and the incorrect L-BFGS label (the callback counts iterations of whichever minimizer method is in use). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Per review on #133 ('or not args.noHessian' should be removed): with --externalPostfit --noFit the covariance is taken from the loaded fitresult again, as before. The recompute also failed with a non-positive-definite Hessian on the test tensor's Asimov workflow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

lucalavezzo added 2 commits April 30, 2026 09:14

asymmetric traditoinal impacts; unblinding groups

fbd4228

fix merge conflicts

520c9a7

lucalavezzo changed the title ~~asymmetric traditoinal impacts; unblinding groups~~ asymmetric traditional impacts; unblinding groups Apr 30, 2026

lucalavezzo and others added 9 commits April 30, 2026 09:42

fix

b49d09e

global asym impacts

467a745

Merge branch 'luca-dev' of github.com:lucalavezzo/rabbit into global-…

42eac6d

…asym-impacts

improve timinig and logging; --noEDM; minimizer settings via CLI; gau…

843fbbd

…ssian constraints on params in ParamModels

changes for NP scetlib model

b2dd17f

merge

2f9e0c0

merge main

433e6e9

format

2fa3ae5

merge main

6bfa4ff

lucalavezzo mentioned this pull request Jun 11, 2026

Support alpha_s decorrelation across rapidity bins WMass/WRemnants#689

Merged

davidwalter2 reviewed Jun 11, 2026

View reviewed changes

lucalavezzo closed this Jun 12, 2026

lucalavezzo mentioned this pull request Jun 12, 2026

SCETlib NP continuous parameter model WMass/WRemnants#701

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

asymmetric traditional impacts; unblinding groups#133

asymmetric traditional impacts; unblinding groups#133
lucalavezzo wants to merge 11 commits into
WMass:mainfrom
lucalavezzo:luca-dev

lucalavezzo commented Apr 30, 2026

Uh oh!

davidwalter2 left a comment

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

davidwalter2 Jun 11, 2026

Uh oh!

lucalavezzo commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		def _gather_poi_noi_vector(v, noiidxs, nsignal_params=0):
		def _gather_poi_noi_vector(v, noiidxs, nsignal_params=0, nmodel_params=None):

Uh oh!

Conversation

lucalavezzo commented Apr 30, 2026

Uh oh!

davidwalter2 left a comment

Choose a reason for hiding this comment

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

davidwalter2 Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

lucalavezzo commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants