More flexible jet derivation alg ordering, PHYSVAL fix
Some pieces of the jet derivation framework previously functioned by adding algorithms directly to the top sequence instead of an arbitrary configurable sequence. With !44380 (merged) this now causes problems for derivations which don't use the default top sequence for all of their scheduling (e.g. PHYSVAL).
This lets the user specify which sequence the algorithms in question should be added to, and does so for both PHYS(LITE) (which should change nothing in practice but makes for clearer config code) and PHYSVAL (which I believe is currently broken and should be fixed by this).
Tagging @duperrin, who reported the issue, and @sawyer, @cdelitzs, @mswiatlo for their awareness.
Merge request reports
Activity
Thanks a lot @wbalunas! I was able to produce a PHYSVAL test sample (100 evts) with this fix (using a private input xAOD produced yesterday). The log and the sample are here: /afs/cern.ch/work/d/duperrin/public/BTAG-SOFT/AfterJetMetRemoval/MR45007fix/
Although the log report "successful run" (and I was able to draw some b-tagging variables from this test PHYSVAL.root), the log report:
AODtoDAOD 10:47:42 2021-07-08 10:47:35,739 jetalgAntiKt4LCTopo...calib_AntiKt4LCTopo_reco_arj FATAL Could not retrieve xAOD::EventShape DataHandle. AODtoDAOD 10:47:42 2021-07-08 10:47:35,739 jetalgAntiKt4LCTopo...calib_AntiKt4LCTopo_reco_arj ERROR JetCalibTools/Root/JetCalibrationTool.cxx:430 (StatusCode JetCalibrationTool::applyCalibration(xAOD::JetContainer&) const): code FAILURE: initializeEvent(jetEventInfo)
Given the EventInfo message above, I gave it a try also implementing this MR where EventInfo was missing in DAOD_PHYSVAL but this seems to be unrelated to the message above.
Still tagging @jferrand @dhayden @jcatmore for their awareness on this Derivation Framework and PHYSVAL MR fix/thread.
I found the culprit for this new issue...
elif inputType == "LCTopo": inputType == "LCTopoOrigin"Clearly just a typo on my part, the second line should of course have
=instead of==. I'm surprised I didn't get any warnings about that. Anyway, fix incoming shortly.-
Hi @jcatmore -
No, this should only affect PHYSVAL. The reason for the issues with PHYS(LITE) is that they're not backwards-compatible with AODs produced before the switch to FlowElements, and I'm under the impression that these tests etc. are attempting to use new nightlies to run derivations on old AODs.
-Bill
-
I ran successfully AODtoDAO with your last fix for the event shape config (it's a PHYSVAL derivation). The afs directory with the test sample and the new log is upadted:
Py:PyJobTransforms.trfValidateRootFile INFO File DAOD_PHYSVAL.AD.test.BillMR45007.root looks ok.
Thanks a lot @wbalunas for these updates :) I think it's ready to merge.
This merge request affects 3 packages:
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkJetEtMiss
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhys
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhysicsValidation
Affected files list will not be printed in this case
Adding @zmarshal ,@jmellent ,@duperrin ,@jveatch ,@nelsonc ,@dshope ,@ispokhar as watchers
added Derivation JetEtmiss master review-pending-level-1 labels
-
Hi @jcatmore -
I'm not sure if any such r-tags have been made yet (!44380 (merged) needs to be included). It might make sense to make one soon if not.
If it's necessary, it might be possible to put in a work-around to support backwards compatibility for derivations. That would probably be in the form of a new flag that tells the config to expect the old PFO EDM instead of FlowElements. But for tests etc. we probably want to be using AODs that are representative of what we're currently doing in reconstruction.
-Bill
Hi Bill,
Do you have a sense of what AODs won't be useable in the derivation framework? Are we still going to be able to make derivations from release 21 AODs? Early release 22 AODs? None?
I think having such a configuration switch would be very useful. I hope it's just a matter of input file peeking and a couple python lines.
Best, Zach
-
Hi Zach -
The way it currently is, the answer is "only R22 AODs made using releases starting now". I'm now realizing this could cause more issues than I anticipated though (it didn't occur to me that people would be interested in making R22 DAODs from R21 or early R22 AODs once the current PhysVal effort is finished).
So I'll go ahead and look into adding some support for this in the jet derivation framework. I think it should be manageable, and I might actually be able to avoid needing a flag at all by just checking which version of the PFO containers is in the input file.
-Bill
Hi all,
unfortunately this discussion has gotten split up across various JIRAs, MRs and private threads so it got a bit confusing.
Just to be clear - as far as I understood things, there's no intention to process r21 AODs with r22 derivation framework. I don't think we ever tried this.
For reading older r22 AODs, it might be worth having some kind of PFO check to side-step this issue, as long as it doesn't over complicate things too much. But unless I've missed a major part of the plan, I didn't think we needed to arrange to process r21 AODs in r22. I assumed that we'll need to continue to maintain the r21 derivation caches for a long while yet.
Cheers,
James.
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals 
cmake make 
required tests optional tests Full details available on this CI monitor view
Hi All, relating to one of the comments above, I submitted a request for no pile-up sample A AODs with the 7th July nightly here in case they would be useful: https://prodtask-dev.cern.ch/prodtask/inputlist_with_request/38369/
-
This merge request affects 3 packages:
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkJetEtMiss
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhys
- PhysicsAnalysis/DerivationFramework/DerivationFrameworkPhysicsValidation
Affected files list will not be printed in this case
Adding @zmarshal ,@jmellent ,@duperrin ,@jveatch ,@nelsonc ,@dshope ,@ispokhar as watchers