Skip to content

Commit

Permalink
fix: Handle importing XML measurements with no POI attribute (#1933)
Browse files Browse the repository at this point in the history 
* Add runtime check for missing POI in XML measurements. XML measurements are not
necessarily required to have a POI, but we require it, so need to fix coverage.
* Add test and test XML files for missing POI in XML configs.
  • Loading branch information
@kratsg
kratsg authored Aug 10, 2022
1 parent 18bc4ec commit 1ed7003
Show file tree
Hide file tree
Showing 5 changed files with 207 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/pyhf/readxml.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,18 @@ def process_measurements(toplvl, other_parameter_configs=None):
lumi = float(x.attrib['Lumi'])
lumierr = lumi * float(x.attrib['LumiRelErr'])

measurement_name = x.attrib['Name']

poi = x.find('POI')
if poi is None:
raise RuntimeError(
f"Measurement {measurement_name} is missing POI specification"
)

result = {
'name': x.attrib['Name'],
'name': measurement_name,
'config': {
'poi': x.findall('POI')[0].text.strip(),
'poi': poi.text.strip() if poi.text else '',
'parameters': [
{
'name': 'lumi',
Expand Down
13 changes: 13 additions & 0 deletions tests/test_import.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,16 @@ def test_import_noChannelData(mocker, datadir):
with pytest.raises(RuntimeError) as excinfo:
pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
assert 'Channel channel1 is missing data. See issue #1911' in str(excinfo.value)


def test_import_missingPOI(mocker, datadir):
_data = [0.0]
_err = [1.0]
mocker.patch('pyhf.readxml.import_root_histogram', return_value=(_data, _err))

basedir = datadir.joinpath("xmlimport_missingPOI")
with pytest.raises(RuntimeError) as excinfo:
pyhf.readxml.parse(basedir.joinpath("config/example.xml"), basedir)
assert 'Measurement GaussExample is missing POI specification' in str(
excinfo.value
)
160 changes: 160 additions & 0 deletions tests/test_import/xmlimport_missingPOI/config/HistFactorySchema.dtd
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@

<!-- The top level combination spec -->
<!-- OutputFilePrefix: Prefix to the output root file to be created (inspection histograms) -->
<!-- Mode: Type of the analysis -->
<!ELEMENT Combination (Function*,Input+,Measurement*)>
<!ATTLIST Combination
OutputFilePrefix CDATA #REQUIRED
Mode CDATA #IMPLIED
>

<!-- Input files detailing the channels. One channel per file -->
<!ELEMENT Function EMPTY>
<!ATTLIST Function
Name CDATA #REQUIRED
Expression CDATA #REQUIRED
Dependents CDATA #REQUIRED
>

<!-- Input files detailing the channels. One channel per file -->
<!ELEMENT Input (#PCDATA) >

<!-- Configuration for each measurement -->
<!-- Name: to be used as the heading in the table -->
<!-- Lumi: the luminosity of the measurement -->
<!-- LumiRelErr: the relative error known for the lumi -->
<!-- BinLow: the lowest bin number used for the measurement (inclusive) -->
<!-- BinHigh: the highest bin number used for the measurement (exclusive) -->
<!-- Mode: type of the measurement (a closed list of ...) -->
<!-- ExportOnly: if "True" skip fit, only export model -->
<!ELEMENT Measurement (POI,ParamSetting*,ConstraintTerm*) >
<!ATTLIST Measurement
Name CDATA #REQUIRED
Lumi CDATA #REQUIRED
LumiRelErr CDATA #REQUIRED
BinLow CDATA #IMPLIED
BinHigh CDATA #IMPLIED
Mode CDATA #IMPLIED
ExportOnly CDATA #IMPLIED
>

<!-- Specify what you are measuring. Corresponds to the name specified in the construction
of the model in the channel setup. Typically the NormFactor for xsec measurements -->
<!ELEMENT POI (#PCDATA) >

<!-- Specify what parameters are fixed, or have particular value -->
<!-- Val: set the value of the parameter -->
<!-- Const: set this parameter constant -->
<!ELEMENT ParamSetting (#PCDATA)>
<!ATTLIST ParamSetting
Val CDATA #IMPLIED
Const CDATA #IMPLIED
>

<!-- Specify an alternative shape to use for given constraint terms (Gaussian is used if this is not specified) -->
<!-- Type: can be Gamma or Uniform -->
<!-- RelativeUncertainty: relative uncertainty on the shape -->
<!ELEMENT ConstraintTerm (#PCDATA)>
<!ATTLIST ConstraintTerm
Type CDATA #REQUIRED
RelativeUncertainty CDATA #IMPLIED
>

<!-- Top element for channels. InputFile, HistoName and HistoPath
can be set at this level in which case they will become defaul to
all subsequent elements. Otherwise they can be set in individual
subelements -->
<!ELEMENT Channel (Data*,StatErrorConfig*,Sample+)>
<!-- InputFile: input file where the input histogram can be found (use abs path) -->
<!-- HistoPath: the path (within the root file) where the histogram can be found -->
<!-- HistoName: the name of the histogram to be used for this (and following in not overridden) item -->
<!ATTLIST Channel
Name CDATA #REQUIRED
InputFile CDATA #IMPLIED
HistoPath CDATA #IMPLIED
HistoName CDATA #IMPLIED
>

<!-- Data to be fit. If you don't provide it, Asimov data will be created -->
<!-- InputFile: any item set here will override the configuration for the subelements.
For this element there is no sublemenents so the setting will only have local effects -->
<!ELEMENT Data EMPTY>
<!ATTLIST Data
InputFile CDATA #IMPLIED
HistoPath CDATA #IMPLIED
HistoName CDATA #IMPLIED
>

<!ELEMENT StatErrorConfig EMPTY>
<!ATTLIST StatErrorConfig
RelErrorThreshold CDATA #IMPLIED
ConstraintType CDATA #IMPLIED
>


<!-- Sample elements are made up of systematic variations -->
<!ELEMENT Sample (StatError | HistoSys | OverallSys | ShapeSys | NormFactor | ShapeFactor)*>
<!ATTLIST Sample
Name CDATA #REQUIRED
InputFile CDATA #IMPLIED
HistoName CDATA #IMPLIED
HistoPath CDATA #IMPLIED
NormalizeByTheory CDATA #IMPLIED
>

<!-- Systematics for which the variation is provided by histograms -->
<!ELEMENT StatError EMPTY>
<!ATTLIST StatError
Activate CDATA #REQUIRED
HistoName CDATA #IMPLIED
InputFile CDATA #IMPLIED
HistoPath CDATA #IMPLIED
>

<!ELEMENT HistoSys EMPTY>
<!ATTLIST HistoSys
Name CDATA #REQUIRED
InputFile CDATA #IMPLIED
HistoFileHigh CDATA #IMPLIED
HistoPathHigh CDATA #IMPLIED
HistoNameHigh CDATA #IMPLIED
HistoFileLow CDATA #IMPLIED
HistoPathLow CDATA #IMPLIED
HistoNameLow CDATA #IMPLIED
>

<!-- Systematics for which the variation is provided by simple overall scaling -->
<!ELEMENT OverallSys EMPTY>
<!ATTLIST OverallSys
Name CDATA #REQUIRED
High CDATA #REQUIRED
Low CDATA #REQUIRED
>

<!-- Systematics for which the variation is provided by simple overall scaling -->
<!ELEMENT ShapeSys EMPTY>
<!ATTLIST ShapeSys
Name CDATA #REQUIRED
HistoName CDATA #REQUIRED
HistoPath CDATA #IMPLIED
InputFile CDATA #IMPLIED
ConstraintType CDATA #IMPLIED
>

<!-- Scaling factor, which may be the parameter of interest for cross section measurements-->
<!ELEMENT NormFactor EMPTY>
<!ATTLIST NormFactor
Name CDATA #REQUIRED
Val CDATA #REQUIRED
High CDATA #REQUIRED
Low CDATA #REQUIRED
Const CDATA #IMPLIED
>


<!-- Systematics for which the variation is provided by simple overall scaling -->
<!ELEMENT ShapeFactor EMPTY>
<!ATTLIST ShapeFactor
Name CDATA #REQUIRED
>

8 changes: 8 additions & 0 deletions tests/test_import/xmlimport_missingPOI/config/example.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE Combination SYSTEM 'HistFactorySchema.dtd'>
<Combination OutputFilePrefix="./results/example" >
<Input>./config/example_channel.xml</Input>
<Measurement Name="GaussExample" Lumi="1." LumiRelErr="0.1" >
<!--<POI>SigXsecOverSM</POI>-->
<ParamSetting Const="True">Lumi alpha_syst1</ParamSetting>
</Measurement>
</Combination>
16 changes: 16 additions & 0 deletions tests/test_import/xmlimport_missingPOI/config/example_channel.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<!DOCTYPE Channel SYSTEM 'HistFactorySchema.dtd'>
<Channel Name="channel1" InputFile="./data/example.root" >
<Data HistoName="data" HistoPath="" />
<Sample Name="signal" HistoPath="" HistoName="signal">
<OverallSys Name="syst1" High="1.05" Low="0.95"/>
<NormFactor Name="SigXsecOverSM" Val="1" Low="0." High="3." />
</Sample>
<Sample Name="background1" HistoPath="" NormalizeByTheory="True" HistoName="background1">
<StatError Activate="True" HistoName="background1_statUncert" />
<OverallSys Name="syst2" Low="0.95" High="1.05"/>
</Sample>
<Sample Name="background2" HistoPath="" NormalizeByTheory="True" HistoName="background2">
<StatError Activate="True" /> <!-- Use Default Histogram Errors as input to StatError -->
<OverallSys Name="syst3" Low="0.95" High="1.05"/>
</Sample>
</Channel>

0 comments on commit 1ed7003

Please sign in to comment.