cms-l1t-offline / cmssw

CMS Offline Software
cms-sw.github.io/cmssw
Apache License 2.0
17 stars 26 forks source link

HLT/L1T Interface Update #103

Open mulhearn opened 8 years ago

mulhearn commented 8 years ago

We are deprecating L1Extra for Stage-2. This requires updates to the HLT/L1T interface.

The topic branch for this issue is here:

https://github.com/cms-l1t-offline/cmssw/tree/l1t-hlt-dev-CMSSW_8_0_0_pre4

The testing recipe is:

cmsrel CMSSW_8_0_0_pre4
cd CMSSW_8_0_0_pre4/src
cmsenv
git cms-merge-topic cms-l1t-offline:l1t-hlt-dev-CMSSW_8_0_0_pre4
scram b -j 8
cmsRun L1Trigger/L1TCommon/test/devHLT.py

Currently the main changes are to

The test code runs L1T and the new HLT module, however, including the module in standard HLT sequences still leads to crashes.

mulhearn commented 8 years ago

Do I understand correctly that you do not need to re-emulate prescales online? If so, we'll simply make those parameters go away for you, and provide a separate configuration in case you wish to prepare L1T input (with simulated prescales). Will this work?

mulhearn commented 8 years ago

Note that we do not yet have O2O or even a condition defined yet for uGT pre-scales. This was advertised in TSG and we were told a recipe involving local config file for prescale emulation will suffice for now.

Martin-Grunewald commented 8 years ago

@mulhearn

Concerning prescale, for case (a) we assume the L1T prescales have already been applied, online for real data in the L1T hardware, and the raw data containing L1A events is then pushed into HLT. (For MC, all prescales are usually set to 1 at L1T and HLT so as to not waste precious MC events).

Now, at HLT we would just need to know WHICH L1T prescale column was used (as we have the same number of prescale columns for HLT prescales and the L1T prescale column index is use to select the HLT prescale column index). The HLT prescaler extracts this info from data, namely:

process.hltPreBTagCSV07 = cms.EDFilter( "HLTPrescaler",
    L1GtReadoutRecordTag = cms.InputTag( "hltGtDigis" ),
    offset = cms.uint32( 0 )
)

with C++ code:

edm::Handle<L1GlobalTriggerReadoutRecord> handle;
      iEvent.getByToken(gtDigiToken_,handle);
      if (handle.isValid()) {
        prescaleSet_ = handle->gtFdlWord().gtPrescaleFactorIndexAlgo();

from the old hltGtDigis module process.hltGtDigis = cms.EDProducer( "L1GlobalTriggerRawToDigi",. We'd need a replacement for that functionality!

For the L1T itself, when one runs L1T on GEN-SIM, then I agree one would need to be able to set and apply L1 prescales, or when trying to make a new L1skim of events from some unbiased zero(min)Bias data sample,but again I suppose this would be done much earlier than the GtProducer instance run inside the HLT. It would be part of the DIGI or L1 or SKIM sequence, no?

During L1REPACK (of real data) we'd also NOT re-apply any L1T prescales (the data is already L1T prescaled!) So I guess we'd need a switch or default file switching all prescales to 1. Well OK, L1REPACK could be part of a SKIM so it is possible we'd want to apply some new L1T prescales when running L1REPACK on some older zero(min)Bias data.

It used to be that L1 prescales could be set via ESProducers or so, with a flag indicating whether they are used or not.

fwyzard commented 8 years ago

as @Martin-Grunewald said, we do not apply prescales to the L1 inside the HLT.

The hltGtStage2ObjectMap is expected to produce the objects map matching the L1 decision that has already happened (in the hardware, or in the MC, or in the emulation, etc.), so it should never apply prescales or masking. My understanding of how the chain works is

If we need to run any re-emulation or prescaling, it will not be done by the hltGtStage2ObjectMap, but by re-running the L1 emulator in the first place.

Martin-Grunewald commented 8 years ago

@rekovic Any progress in fixing the issue with the missing Refs? Otherwise, please make a PR and we ask the FwEDM team for help. Actually, please make a PR anyway :)

rekovic commented 8 years ago

@Martin-Grunewald
I just made a TEST only PR https://github.com/cms-sw/cmssw/pull/13262

A simply Producer is provided in the sequence HLTrigger/HLTfilters/test/devHLT_Stage2.py after the HLTL1T seed to test for Ref Simple registering of Ref to the Event crashes. Trying to look deeper into the BXvector, and performing tests.

mulhearn commented 8 years ago

Ooof... it seems we still have work to adapt bx-vector to play nice with edm::Ref.

mulhearn commented 8 years ago

@rekovic I believe I have a fix for this. Let me test more and then I will send you a commit to cherry-pick.

mulhearn commented 8 years ago

Yes, its working:

++l1tMuonBXVectorRefs "bxVector" "" "L1TMuonEmulation" (productId = 2:30)
++l1tMuonBXVectorl1tMuonl1tMuonBXVectorl1tMuonedmrefhelperFindUsingAdvanceedmRef "bxVector" "" "L1TMuonEmulation" (productId = 2:31)

I'll push the fix shortly so you can start working with it, but I'd still like to confirm I can read the read the payload properly in a downstream module.

rekovic commented 8 years ago

@mulhearn I think I have a fix also (I conIt took a long time to compile) Had to add missing statements with edm::Wrapper for for Refs in DataFormats/L1Trigger/src/classes_def.xml. My test module now runs on Refs.

Martin-Grunewald commented 8 years ago

@mulhearn @rekovic Please make sure your two fixes are the same or otherwise clarify any differences....

mulhearn commented 8 years ago

Indeed, thats all that was missing. Here's a commit that updates classes and classes_def for all L1T objects: 287f4b573de4fa5aae8df20b0739bd81d6378ec3

mulhearn commented 8 years ago

This was tested with a module that writes every L1T type of Ref / RefVector. Haven't tested yet that we can read these back.

Martin-Grunewald commented 8 years ago

...and update the PR :) Thanks a lot! BTW, I see the PR applies cleanly to 800pre6 but shows merge problems in github (which tests against the most recent 80X IB). So eventually the merge issues need to be fixed to that the code can go in.

mulhearn commented 8 years ago

Yeah, it needs a rebase... we'll clean that up when we have something working all the way through.

In fact, @rekovic, if you stop by I'll help you rebase to an IB already now, which should substantially reduce your rather extensive compiling...

rekovic commented 8 years ago

@mulhearn Sounds good!

Martin-Grunewald commented 8 years ago

@mulhearn I see you have closed #13233. So where will this part go?

mulhearn commented 8 years ago

@Martin-Grunewald I @mentioned you at cms-sw#13271 which looks to be working great to me. Now we just need cleanup of Gt emulator interface as you requested plus demonstration that Vladimir's seed filter is properly writing objects to event record.

rekovic commented 8 years ago

@Martin-Grunewald @mulhearn Now I am able to store references to l1t objects in the Event, so that is all fine.
But at the moment, the TFOWR still does not keep them. Investigating further.

rekovic commented 8 years ago

@Martin-Grunewald @mulhearn

Solved 9ad317927cab3c25d9ec88545dc5751cdc98ed17. Now TFOWR is retaining the Refs. Swap methods for Refs were missing in TriggerRefCollections. Will provide a PR. Example printout:

HLTL1TSeed: seed logical expression = L1_SingleS1Jet36 AND L1_SingleEG10

L1Mu seeds: 0

L1EG seeds: 1 L1EG pt = 11.5 eta = -4.4805 phi = 1.566

L1Jet seeds: 2 L1Jet pt = 45 eta = -4.437 phi = 1.566 L1Jet pt = 44.5 eta = 0.261 phi = -0.976185

L1Tau seeds: 0

L1EtSum ETT seeds: 0

L1EtSum HTT seeds: 0

L1EtSum ETM seeds: 0

L1EtSum HTM seeds: 0

HLTL1Seed::hltFilter(..., trigger::TriggerFilterObjectWithRefs & filterproduct) filterproduct.l1tjetIds().size() = 2 filterproduct.l1tjetRefs().size() = 2 %MSG-d TriggerSummaryProducerRaw: TriggerSummaryProducerRAW:hltTriggerSummaryRAW 13-Feb-2016 19:32:10 CET Run: 1 Event: 1 TriggerSummaryProducerRAW.cc:79 Number of filter objects found: 1 %MSG 0 InputTag: label = hltL1TSeed, instance = , process = L1SEQS Sizes: 1/0 2/0 3/0 4/0 5/0 6/0 7/0 8/0 9/0 A/0 B/0 C/0 D/0 E/0 F/0 G/0 I/0 J/1 K/2 L/0 M/0 TriggerSummaryProducerRaw::addFilterObjects( ) fobs[ifob]->l1tmuonIds().size() = 0 fobs[ifob]->l1tmuonRefs().size() = 0 TriggerSummaryProducerRaw::addFilterObjects( ) fobs[ifob]->l1tegammaIds().size() = 1 fobs[ifob]->l1tegammaRefs().size() = 1 TriggerSummaryProducerRaw::addFilterObjects( ) fobs[ifob]->l1tjetIds().size() = 2 fobs[ifob]->l1tjetRefs().size() = 2 TriggerSummaryProducerRaw::addFilterObjects( ) fobs[ifob]->l1ttauIds().size() = 0 fobs[ifob]->l1ttauRefs().size() = 0 TriggerSummaryProducerRaw::addFilterObjects( ) fobs[ifob]->l1tetsumIds().size() = 0 fobs[ifob]->l1tetsumRefs().size() = 0 Number of filter objects packed: 1

rekovic commented 8 years ago

@Martin-Grunewald @mulhearn Made PR #13275. Tomorrow I can add a simple Example HLT Producer that uses L1T Refs from TFOWR That would probably be useful.

mulhearn commented 8 years ago

@Martin-Grunewald please see cms-sw#13271 for a substantially cleaned up param layer interface between HLT/L1T, based on our discussions. That pull request also provides the requested L1REPACK capability.

mulhearn commented 8 years ago

The new HLT config fragments now look like this: https://github.com/cms-l1t-offline/cmssw/blob/l1t-hlt-80x/L1Trigger/L1TCommon/test/runHLT.py#L36

mulhearn commented 8 years ago

@Rekovic, its looking good, but I think one thing is missing that I didn't appreciate until our discussion above (regarding Masking). The re-Emulated GT Object Map will not have either prescales or masks applied. Instead, you should use the "actual" GT output algorithm bits (unpacked from RAW) which might differ from the ones recorded in the re-Emulated GT Object Map due to pre-scales and masks. I think this is fine for first pass though and we should focus on integration... I don't think pre-scales are that important for cosmics! But perhaps we could already now provide the config hook to specify the GT input tag ("hltGtStage2Digis") even if your code ignores it for now.

mulhearn commented 8 years ago

(comment edited for typos that made that confusing... probably best to read that on github)

rekovic commented 8 years ago

@mulhearn The config hook for GT Object Map already exists, and it is used https://github.com/cms-l1t-offline/cmssw/blob/9ad317927cab3c25d9ec88545dc5751cdc98ed17/HLTrigger/HLTfilters/test/devHLT_Stage2.py#L119-L128

mulhearn commented 8 years ago

@rekovic: I'm talking about a hook for the unpacked GT output which is different from re-Emulated GT Object Map. Let's Skype...

Martin-Grunewald commented 8 years ago

@mulhearn @rekovic @fwyzard

On another topic: the l1extra migration of downstream modules: in 800pre6, there are 128 files referring to a type from the l1extra::L1* namespace according to

cd CMSSW_8_0_0_pre6/src
cmsenv
rehash
git grep 'l1extra::L1' | cut -d: -f1 | sort -u

Either this is migrated centrally by L1T, or, since the L1T team is very busy, by TSG/POGs/PAGs.

If it is the latter, we would need a translation table (eg, is the migration consisting of only changes in the type while all acessor methods would work with the same name or so, etc.).

We'd also need to clarify if the old l1extra-based code should be kept alive, or if we can go for a clean break: At one time in the past it was even mentioned to construct l1extra collections from stage-2 output simply to be able to decouple this downstream migration....

fwyzard commented 8 years ago

Either this is migrated centrally by L1T, or, since the L1T team is very busy, by TSG/POGs/PAGs. If it is the latter, we would need a translation table (eg, is the migration consisting of only changes in the type while all accessor methods would work with the same name or so, etc.).

This needs to happen within the next two weeks, so I guess it has to be TSG, POGs and DPGs. I don't think PAGs own such code, but I did not explicitly check.

We'd also need to clarify if the old l1extra-based code should be kept alive, or if we can go for a clean break: At one time in the past it was even mentioned to construct l1extra collections from stage-2 output simply to be able to decouple this downstream migration....

Given the current time scale (i.e. calendar year 2016), we should keep the old code unchanged, and add new versions of the affected modules dedicated to uGT/Stage 2. Once that s in place, we can discuss with Physics and Offline whether we need to maintain the capability to run over GT/Legacy/Stage 1 data or not.

mulhearn commented 8 years ago

I'd prefer to write legacy -> upgrade converters and switch to using the upgrade formats everywhere in one clean switch, that preserves legacy/stage-1/stage-2 functionality. The L1T software can handle any era in the same release.

Certainly our intention was for the new objects to drop in as close as possible to l1extra.

I'll have to look carefully through a few examples before I can provide a detailed recipe, or ideally maybe even a script that can handle most cases.

I can see the appeal, but I guess I just hate the idea of an upgrade -> legacy converter. Seems like going backwards and wasting time. But maybe after having a closer look I'll change my mind...

mulhearn commented 8 years ago

PR cms-sw#13277 merges Rekovic's latest with my updates for standard sequences and parameter cleanup.

mulhearn commented 8 years ago

(@rekovic you can either put your new work on that branch, or I'll just cherry-pick any commits past 2d0b01422d23781e09ad0ca74519b74bbd121548 from your current branch.)

mulhearn commented 8 years ago

@Martin-Grunewald @fwyzard @rekovic my impression is things are settled reasonably on the HLT/L1T front and I can turn close to my full attention now to the downstream modules, starting with making a Legacy->Upgrade conversion tool. Is this a correct assessment, or are we still missing something in critical path for HLT?

Martin-Grunewald commented 8 years ago

I'd agree. Let's hope the PRs get integrated quickly.

But why are you looking at a Legacy-to-Upgrade conversion tool? The upstream L1T is already putting out the upgrade dataformat objects (replacing the old L1extra), so if anything we'd need a upgrade to legacy "down" conversion as long as the down stream users of the old l1extra objects are not yet converted/extended/duplicated to use the upgrade objects (of course we could skip that and copy/edit new modules reading the upgrade dataformat objects of all plugins consuming the old l1extra)....

fwyzard commented 8 years ago

I'm strongly against down-converting from Stage 2 to Legacy format: if we allow people not to migrate their code now, it will take forever to happen (unless we do it ourselves).

I don't know if it is useful to up-convert from Legacy to Stage 2. The use cases I can think of are

@mulhearn , I'm sorry to say a bit lost on the latest&greatest status of the L1 software. On the algorithm side, is there anything that needs work on your side ? On the CMSSW software side, do we have the code that makes use of the UTM library yet ?

Martin-Grunewald commented 8 years ago

@fwyzard ad 1) and ad 2) we'd run L1REPACK anyway, no? I do not think 2015 L1T and 2016 HLT makes much interest... ???

fwyzard commented 8 years ago

@Martin-Grunewald I agree 1. (run 2016 HLT on 2015 data as-is) does not make much sense. About 2. (re-analyse) I meant offline analyses; for example, I do not know what are the status and plans for accessing the GT/uGT information in the mini-AOD.

fwyzard commented 8 years ago

By the way, @bortigno mentioned that the current version of the seeding module would not take into account the L1 masks and prescales. Is it the issue that the filter was reading the L1 results from the re-emulated collection ? Is it still the case, or did @rekovic fix it this morning ?

Martin-Grunewald commented 8 years ago

@fwyzard - fine but this should not have any priority for L1T/HLT/TSG whatsoever given that we are late .... IOW our highest priority should be to clone-and-modify the modules currently using the old l1extra.

fwyzard commented 8 years ago

@fwyzard - fine but this should not have any priority for L1T/HLT/TSG whatsoever given that we are late .... IOW our highest priority should be to clone-and-modify the modules currently using the old l1extra.

if you mean the offline analysis stuff, I completely agree. Indeed, the point of my comment was that the up/down conversion between Legacy and Stage 2 should not be among the priorities.

mulhearn commented 8 years ago

@fwyzard, as a technical matter we (l1T) would still like the legacy conversion to upgrade format (at least for muons) so that we can do compare performance of Stage-1 and Stage-2 directly using the upgrade formats (right now we still use l1extra for Stage-1). Agree this is not highest priority.

Also agree that if we provide a way out of deprecating l1extra, it will probably never happen downstream. But perhaps time is too short now... we do need fully operational software at the end of day.

As to your question on software status: Current version of seeds filter bases the seed selection on the re-emulated decision, which does not include masks and prescales. Vladimir is working on update to use the unpacked (either from "real" GT, or "emulated with masks and prescales applied in production L1 simulation" GT) algorithm bits instead of those stored in Object map. The hook for specifying the input is already added however, so this will be a drop in fix. We were assuming this is not therefore in critical path for 80x, but Vladimir is working on it full speed in either case.

On the algorithm side: we still do not have 1x1 HF TPs in dev recipe branch. We also still do not have calo confirmation that sums are correct (though what is in dev-recipes is their latest and greatest...) They are still investigating.

The global emulator in dev-recipe branch uses UTM.

rekovic commented 8 years ago

@bortigno @fwyzard No - to your 2nd question...Fix for the seeding module to use masks and prescales is NOT in yet in this (3.0) version in the PR. Took us some time to rebase PR this morning. Yes - to your 1st question...The seeding gets the L1 results from the ObjectMap made by GT emulator (hence does not know of the masks and prescales) and should get it from RAW GT.

fwyzard commented 8 years ago

Also agree that if we provide a way out of deprecating l1extra, it will probably never happen downstream. But perhaps time is too short now... we do need fully operational software at the end of day.

Sorry, but I have seen way too many times temporary solutions become the norm. So I'd rather have you spend some time writing down the instructions for the POGs to migrate from L1ExtraParticles to the new candidate formats, than writing a converter that allows them to postpone the work :-) The Muons POG is alredy working on it, and we'll see during the workshop the status from the other ones, and prod them as necessary.

Current version of seeds filter bases the seed selection on the re-emulated decision, which does not include masks and prescales. Vladimir is working on update to use the unpacked (either from "real" GT, or "emulated with masks and prescales applied in production L1 simulation" GT) algorithm bits instead of those stored in Object map. The hook for specifying the input is already added however, so this will be a drop in fix. We were assuming this is not therefore in critical path for 80x, but Vladimir is working on it full speed in either case.

As far as I can tell, getting the right results from the filter is necessary both for the MWGR, and for the HLT developments. So, yes, from my point of view it is in the critical path for 8.0.0 .

Martin-Grunewald commented 8 years ago

@mulhearn Could you please tell me where the (new) L1T menus themselves are located? AFAIK they are only in the form of xml files? Where in CMSSW? Also, do you have L1T menus (xml files or other) corresponding to the three use cases listed here: pp: L1Menu_Collisions2015_25nsStage1_v5 HIon: L1Menu_CollisionsHeavyIons2015_v5_mc ppref: L1Menu_Collisions2015_5TeV_pp_reference_v5_mc How quickly can they be produced (even if they are only just doing the same as before)? We'd need those to keep those corresponding HLT menus running for development.

Martin-Grunewald commented 8 years ago

@rekovic What is the status of the template specialisation in TriggerSummaryAOD? TriggerSummaryProducerAOD::fillTriggerObject and TriggerSummaryProducerAOD::fillFilterObjectMember

mulhearn commented 8 years ago

The current menu is here: L1Trigger/L1TGlobal/data/Luminosity/startup/L1Menu_Collisions2015_25nsStage1_v7_uGT.xml I'm not sure what you are asking for exactly in the next part. To reliably operate L1T emulation for Stage-1 (2015) we need to be in the 2015 Era, and will therefore run the legacy GT emulator in our standard sequences, the same as before. If you are asking for an L1Menu for the uGT that corresponds to the 2015 menu (even though the inputs will be different) then I'll have to ask the GT folks.

Martin-Grunewald commented 8 years ago

@mulhearn

What I mean by the other menus is the following: we'll also update the HIon and PRef HLT menus to use the stage-2 L1T (ie, your unpacking sequence). We have to do so as all HLT menus live in the same master table and several paths are shared between all three HLT menus.

Hence for those use cases we would then use L1REPACK on old data (running stage2 L1T) and feed that to HIon and PRef HLT menus, same way as for the pp menu. This will allow further development of those HLT menus for data taking later this year. But then we'd need the corresponding L1T menus in a "stage-2" format, like the pp menu you already provide (the above v7 in xml). For now it should for each case really be just a menu with the same L1 algorithms (bit names) as in the v5 menus used for HIon and PRef, using some stage-2 equivalent.

mulhearn commented 8 years ago

Excellent, I understand, thank you for the clarification! I'll get back to you with either the XML files or a timescale for delivering them...

fwyzard commented 8 years ago

@Martin-Grunewald , I agree we want to have the uGT version of the p-Pb and PbPb menus as well - but we should not let their (lack of) availability get in the way of the migration to uGT of the GRun menu.

I'd rather have the HIon and PIon workflows broken for a few weeks, than delay the GRun migration.

rekovic commented 8 years ago

@Martin-Grunewald The EtSum objects all store their scalar values as et. So I don't think there is any need for specialisation of templated functions TriggerSummaryProducerAOD::fillTriggerObject and TriggerSummaryProducerAOD::fillFilterObjectMember.

On a similar, but a slightly different note, I have a bug fix for the Interface seeding of EtSums, 7f99810095530634ccce9a95a25624333bce5fd0. The bug was always making seeds with ETT object. I will soon have a PR.

@fwyzard On top of the SumEt bug fix, I am trying to get the masking and prescaling reflected in the interface.