Add bTagCalibFile option to makeFTagAnalysisConfig
FTagConfig already has an option to configure the b-tagging CDI file, but that option is not available in makeFTagAnalysisConfig, so users can't change the CDI file. This MR adds a bTagCalibFile option to makeFTagAnalysisConfig.
Merge request reports
Activity
Athena AnalysisBase AthAnalysis externals cmake make tests Full details available on this CI monitor view. Check the JIRA CI status board for known problems
288 292 if legacyRecommendations is not None : 289 293 config.setOptionValue ('legacyRecommendations', legacyRecommendations) 290 294 if minPt is not None : 291 295 config.setOptionValue ('minPt', minPt) - Comment on lines 278 to 291
this could be a loop:
280 if btagWP is not None : 281 config.setOptionValue ('btagWP', btagWP) 282 if btagger is not None : 283 config.setOptionValue ('btagger', btagger) 284 if bTagCalibFile is not None: 285 config.setOptionValue ('bTagCalibFile', bTagCalibFile) 286 if generator is not None : 287 config.setOptionValue ('generator', generator) 288 if kinematicSelection is not None : 289 config.setOptionValue ('kinematicSelection', kinematicSelection) 290 if noEffSF is not None : 291 config.setOptionValue ('noEffSF', noEffSF) 292 if legacyRecommendations is not None : 293 config.setOptionValue ('legacyRecommendations', legacyRecommendations) 294 if minPt is not None : 295 config.setOptionValue ('minPt', minPt) 280 for kw, val in kwargs.items(): 281 config.setOptionValue(kw, val)
257 257 selectionName, 258 258 btagWP = None, 259 259 btagger = None, 260 bTagCalibFile = None, 260 261 generator = None, 261 262 kinematicSelection = None, 262 263 noEffSF = None, 263 264 legacyRecommendations = None, 264 265 minPt = None ): Let me preface this comment by saying: I'm very convincible on all of this. I'm not sure I know how a good configuration system ought to look like, and am not that happy with any I designed in the past.
I'd say the same, but I'd go in a different direction. My personal feeling is that we shouldn't use these function parameters, but that those are there mostly for historical reasons. In the actual code examples I give user I use
setOptionValue, and also don't call these functions directly. At the same time as long it doesn't break anything, I'm not opposed either.I do feel fairly strongly though what we do not want to forward declare the options at every layer of the configuration system. There was a conscious decision that at the block level we'd forward declare all tool/algorithm properties as options to create a layer of separation, but we don't want to do this at any other level. Having the
**kwargswould avoid that, though as said before I'm not sure it's a good idea to have the user pass options that way.
removed analysis-review-required label
added analysis-review-approved label
Resolved by Ryan Quinn@rquinn Just a note that with the newly available (and preferred?) YAML config approach to scheduling these config blocks, one can directly edit the
bTagCalibFileoption in the YAML config.- Last reply by Ryan Quinn
added review-user-action-required label
removed review-pending-level-1 label
Adding review-user-action-required while there are unresolved threads. If they have been resolved be sure to click "Resolve thread" on each of them and we can continue with the review. Tom - L1 shifter
-
A larger rework of configuration options as discussed between @dguest and @krumnack above is probably beyond the scope of this MR.
At this point, after learning that the YAML approach works with TopCPToolkit and avoids this issue (thanks @ravinab!), I'm going to go ahead and close this MR. Anyone else can feel free to reopen, but I got what I needed :)
