Skip to content
Snippets Groups Projects

More flexible jet derivation alg ordering, PHYSVAL fix

Merged William Keaton Balunas requested to merge wbalunas/athena:jetDeriv-210707 into master

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.

Edited by William Keaton Balunas

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 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.

  • Hi @duperrin ,

    I think the EventShape error messages may be a separate issue. PHYS and PHYSLITE are also crashing with these errors since yesterday's build. I guess a JIRA will follow on this shortly.

    James.

  • 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.

  • added 1 commit

    • 2e912ee2 - Fix typo in event shape config

    Compare with previous version

  • Hi @wbalunas ,

    is this typo also the cause of the issues in PHYS(LITE) that @elmsheus has reported?

    Thanks!

    James.

  • 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

  • Hi Bill,

    ah, ok - thanks for the clarification. Do you know from which r-tag the AODs included the FlowElements, so we can stop trying to use these old AODs?

    Cheers,

    James.

  • 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.

  • William Keaton Balunas marked this merge request as ready

    marked this merge request as ready

  • 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

  • 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.

  • :negative_squared_cross_mark: CI Result FAILURE (hash 2e912ee2)

    Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon
    externals :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    cmake :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    make :white_check_mark: :white_check_mark: :o: :white_check_mark: :white_check_mark: :white_check_mark:
    required tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:
    optional tests :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark: :white_check_mark:

    Full details available on this CI monitor view
    :white_check_mark: Athena: number of compilation errors 0, warnings 0
    :white_check_mark: AthSimulation: number of compilation errors 0, warnings 0
    :o: AthGeneration: number of compilation errors 1, warnings 0
    :white_check_mark: AnalysisBase: number of compilation errors 0, warnings 0
    :white_check_mark: AthAnalysis: number of compilation errors 0, warnings 0
    :white_check_mark: DetCommon: number of compilation errors 0, warnings 0
    :pencil: For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 36568]

  • 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/

  • CI failure looks unrelated to me (probably from fastjet update?)

  • Jenkins please retry a build

  • 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

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading