Skip to content

[RF][HS3] Accept "shapefactor" modifier type in HistFactory importer#21213

Merged
guitargeek merged 2 commits intoroot-project:masterfrom
cburgard:shapesys-bugfixes
Apr 27, 2026
Merged

[RF][HS3] Accept "shapefactor" modifier type in HistFactory importer#21213
guitargeek merged 2 commits intoroot-project:masterfrom
cburgard:shapesys-bugfixes

Conversation

@cburgard
Copy link
Copy Markdown
Contributor

@cburgard cburgard commented Feb 9, 2026

The HS3 importer only recognised "shapesys" for ParamHistFunc-based modifiers and rejected the more accurate "shapefactor" type with "modifier of unknown type 'shapefactor'". Accept both spellings, and allow the modifier's "vals" array to be empty (or absent) when "parameters" is provided, as is typical for unconstrained shape factors written by other HS3 producers (e.g. pyhf).

@cburgard cburgard requested a review from guitargeek as a code owner February 9, 2026 16:53
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 9, 2026

Test Results

    22 files      22 suites   3d 11h 2m 54s ⏱️
 3 849 tests  3 846 ✅  1 💤 2 ❌
76 007 runs  75 987 ✅ 18 💤 2 ❌

For more details on these failures, see this check.

Results for commit 9abe2e9.

♻️ This comment has been updated with latest results.

Comment thread roofit/hs3/src/JSONFactories_HistFactory.cxx Outdated
cburgard and others added 2 commits April 27, 2026 00:12
The HS3 importer only recognised "shapesys" for ParamHistFunc-based
modifiers and rejected the more accurate "shapefactor" type with
"modifier of unknown type 'shapefactor'". Accept both spellings, and
allow the modifier's "vals" array to be empty (or absent) when
"parameters" is provided, as is typical for unconstrained shape factors
written by other HS3 producers (e.g. pyhf).
This avoids some unneeded RooStats dependency.
@guitargeek guitargeek changed the title [RF][HS3] Shapesys bugfixes [RF][HS3] Accept "shapefactor" modifier type in HistFactory importer Apr 26, 2026
Copy link
Copy Markdown
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

LGTM! I have also added a unit test for the case that is now supported.

@guitargeek guitargeek dismissed silverweed’s stale review April 27, 2026 07:18

Clang formatting applied

@guitargeek guitargeek merged commit 8c2ca49 into root-project:master Apr 27, 2026
30 of 33 checks passed
@guitargeek guitargeek deleted the shapesys-bugfixes branch April 27, 2026 07:18
@guitargeek
Copy link
Copy Markdown
Contributor

/backport to 6.40

@root-project-bot
Copy link
Copy Markdown

Preparing to backport PR #21213 to branch 6.40 requested by guitargeek

@root-project-bot
Copy link
Copy Markdown

This PR has been backported to branch 6.40: #22068

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants