[RF][HF] Support setting ShapeFactor value and range#20723
Merged
Conversation
Test Results 22 files 22 suites 3d 13h 59m 45s ⏱️ For more details on these failures, see this check. Results for commit fcdcbbe. ♻️ This comment has been updated with latest results. |
Adds a Sample::AddShapeFactor overload that takes the initial value and the range of the ShapeFactor gammas, analogous to AddNormFactor. This is important e.g. for ABCD estimates, where the hard-coded default range can cause convergence problems. The value and range are persisted to and read back from both XML and ROOT files, and a dedicated test covers the full round trips as well as the resulting workspace parameters. The ShapeFactor element in the XML schema (HistFactorySchema.dtd) is updated accordingly. The value and range use the same 'Val' / 'Low' / 'High' attribute and accessor names as NormFactor for consistency. Closes root-project#20697. 🤖 Done with the help of AI for writing the tests.
Sample::PrintXML now writes the "Const" attribute for ShapeFactors, so a ShapeFactor that was marked constant via SetConstant() keeps that state when a Measurement is written to XML and read back. The read side and schema were already in place: **ConfigParser::MakeShapeFactor** parses the "Const" attribute, and the ShapeFactor element declares it in HistFactorySchema.dtd. Only the writer was missing it, so the const-ness was silently lost on a PrintXML round trip. The attribute is always emitted as "True"/"False", matching how other booleans (NormalizeByTheory, StatError's Activate) are written. Unlike NormFactor, whose const-ness is handled at the Measurement level via `<ParamSetting Const="True">`, ShapeFactor carries its own per-bin constant flag (wired into the gamma parameters in HistoToWorkspaceFactoryFast), which is why it needs to be persisted on the element itself. 🤖 Done by AI.
Contributor
Author
|
Backporting also to 6.40 on request by ATLAS (@TomasDado) |
Contributor
Author
|
/backport to 6.40 |
|
Preparing to backport PR #20723 to branch 6.40 requested by guitargeek |
|
This PR has been backported to branch 6.40: #22642 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Sample::AddShapeFactor overload that takes the initial value and the range of the ShapeFactor gammas, analogous to AddNormFactor. This is important e.g. for ABCD estimates, where the hard-coded default range can cause convergence problems.
The value and range are persisted to and read back from both XML and ROOT files, and a dedicated test covers the full round trips as well as the resulting workspace parameters. The ShapeFactor element in the XML schema (HistFactorySchema.dtd) is updated accordingly. The value and range use the same 'Val' / 'Low' / 'High' attribute and accessor names as NormFactor for consistency.
Closes #20697.