Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HLT/L1T Interface Update #103

Open
mulhearn opened this issue Dec 3, 2015 · 336 comments
Open

HLT/L1T Interface Update #103

mulhearn opened this issue Dec 3, 2015 · 336 comments

Comments

@mulhearn
Copy link
Member

mulhearn commented Dec 3, 2015

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

  • update BXVector to work with edm::Ref as needed for HLT usage of L1T objects.
  • implement a new module HLTrigger/HLTfilters/interface/HLTL1TSeed.h to replace HLTrigger/HLTfilters/interface/HLTLevel1GTSeed.h

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

@mulhearn
Copy link
Member Author

mulhearn commented Dec 3, 2015

@mulhearn
Copy link
Member Author

Just merged the packer/unpacker code from GT into the dev recipe branch and it seems to work. So ready to update L1Repack.

Also started updating the HLT filter which selects the seeds which fired specified L1 triggers. Decided that for pass1 the updated module will simply provide all the L1 trigger seeds without selecting based on what trigger was passed. Then, once we have the replacement for L1GtUtils working, we'll extend the HLT filter to use it. There's one additional layer of parsing that handles making arbitrary combinations of L1 trigger bits, but I think that can be recycled from the legacy system.

@mulhearn
Copy link
Member Author

I have a new HLT filter for HLTL1TSeed to replace HLTLevel1GTSeed. At the moment it still doesn't do much except not die. This lets me see what else is downstream that depends on l1extra. The list is fortunately not as long as I feared:

#   modified:   HLTrigger/Muon/src/HLTMuonL1Filter.cc
#   modified:   RecoEgamma/EgammaHLTProducers/interface/HLTRecHitInAllL1RegionsProducer.h
#   modified:   RecoMuon/L2MuonSeedGenerator/src/L2MuonSeedGenerator.cc
#   modified:   RecoTauTag/HLTProducers/src/CaloTowerCreatorForTauHLT.cc

also I simply removed for now the "HLTLevel1Activity.cc" module.

With all of these modules crippled to stop consuming l1extra and output empty collections, the HLT step now runs in standard sequences. I looked into these producers except HLTLevel1Activity and we should be able to just switch in the new formats.

So total tally I can see is one module completely rewritten (first pass almost done) and 5 modules need to modified once this is complete.

@mulhearn
Copy link
Member Author

Oooof... I hope this is close to bottoming out: I still keep finding more changes needed. It seems we haven't hooked up BxVector to work with edm::Ref yet. This functionality is definitely needed to play nice with HLT.

@mulhearn
Copy link
Member Author

BxVector is fixed to work with edm::Ref now...

@mulhearn
Copy link
Member Author

mulhearn commented Jan 8, 2016

@rekovic this should be a good start, I'll start a topic branch that includes my recent progress.

@rekovic
Copy link

rekovic commented Jan 8, 2016

@mulhearn Lets do it.

@mulhearn
Copy link
Member Author

I got a bunch of core dumps while debugging this... I changed all the build files which I don't want to push but I think the correct recipe to just do this centrally is to edit the file:

$CMSSW_RELEASE_BASE//config/BuildFile.xml

to include:

<Flags CXXFLAGS="-g"/>

and this option will applied globally to all checked out packages.

@mulhearn
Copy link
Member Author

I believe one of the problem spots is here:

  DataFormats/HLTReco/interface/TriggerFilterObjectWithRefs.h

which I started but didn't finish debugging:

-      collectionTags_.push_back(collectionTag.encode());
+      std::string tmp = collectionTag.encode();
+      //std::cout << collectionTags_ << "\n";
+      std::cout << "SIZE:  " << collectionTags_.size() << "\n";
+      collectionTags_.push_back(tmp);

@mulhearn
Copy link
Member Author

Leaving out of branch some changes I made to these files, where were just to shut them up by commenting out everything...

#   modified:   HLTrigger/Muon/src/HLTMuonL1Filter.cc
#   modified:   RecoEgamma/EgammaHLTProducers/interface/HLTRecHitInAllL1RegionsProducer.h
#   modified:   RecoMuon/L2MuonSeedGenerator/src/L2MuonSeedGenerator.cc
#   modified:   RecoTauTag/HLTProducers/src/CaloTowerCreatorForTauHLT.cc

@mulhearn
Copy link
Member Author

The initial comment on this githup issue has been updated to include the recipe for compiling and running the new topic branch, and summary of status.

@rekovic
Copy link

rekovic commented Jan 15, 2016

In DataFormats/HLTReco/interface/TriggerRefsCollections.h
there is only one method that adds l1t:: objects.

void addObject(int id, const l1t::MuonRef& ref) {
  l1tmuonIds_.push_back(id);
  l1tmuonRefs_.push_back(ref);
}

Need to add methods for l1t::Egamma, l1t::Jet, l1t::Tau, l1t::EtSum
Is this the whole list of l1t objects?

@mulhearn
Copy link
Member Author

That'll do it!

@rekovic
Copy link

rekovic commented Jan 18, 2016

Made changes to extend functionality. Apart from l1t::Muon, we now have:
EGamma,
Jet,
Tau,
EtSum.

When putting above l1t candidates for HLT seeds, I used the existing HLT trigger types respectively:
TriggerL1NoIsoEG,
TriggerL1CenJet,
TriggerL1TauJet,
TriggerL1ETT.

// Example For EGamma
filterproduct.addObject(trigger::TriggerL1NoIsoEG, myref); // Temp just use NoIsoEG

Not sure if this OK. Need to check with with M.Gruenwald or A.Bocci.

If compiled with

 USER_CXXFLAGS="-g -D=EDM_ML_DEBUG" scram b -j 8

running the test job

cmsRun L1Trigger/L1TCommon/test/devHLT.py

gives output log files critical.log and detailedInfo.log.
detailedInfo.log has output of L1T candidate seeds. PT is currently always zero. Did not look into why this is.

@mulhearn
Copy link
Member Author

Andrea provides a useful set of questions to help organize this effort:

0. summarise the changes to the L1 software with respect to Legacy / Stage 1

My naïve guess is that all the interfaces to the L1 object have changed: new CondFormats, new DataFormats, etc.
I think that it may be helpful to have a brief summary or "translation table", to understand what conditions and objects were used for the Legacy trigger, and what are available now.

|   old C++ object   |   new C++ object   |   what is is / does   |

For example I would find it useful to understand what happens to 
  - the L1GtTriggerMenu conditions object, used to associate the trigger names to the bits; 
  - the L1GtTriggerMask conditions object, used for the trigger masks;
  - the L1GlobalTriggerReadoutRecord object, used to store the trigger results (before / after masks?)
  - the L1GlobalTriggerObjectMapRecord object, used to associate the triggers and the candidates
  - the l1extra::* objects, i.e. the L1 trigger candidates, and the additional quality flags they referenced (e.g. the L1MuGMTExtendedCand use by the l1extra::L1MuonParticle)

Some of these objects are clearly internal to the L1 software - the HLT does not need one to one replacements, just to maintain the overall functionalities - which leads to the next point.


1. identify all CMSSW modules used at HLT that need to be updated

In general terms, HLT uses
  - the L1 trigger decisions,
  - the L1 trigger candidates (the old "extra particles") and their qualities,
  - and the mapping between triggers and candidates (the old "object map").

Given the changes from 0. above, what HLT modules need to be updated, rewritten, or replaced with new modules ?

Basically every HLT path begins by unpacking and reading the L1 results and candidates:
  - L1 unpackers (L1GlobalTriggerRawToDigi, L1TRawToDigi, L1TCaloUpgradeToGCTConverter)
  - L1 producers of the mapping and candidates (L1GlobalTrigger, L1ExtraParticlesProd)
  - L1 "seed" filter, that checks if the given combination of L1 trigger fired, and produces the associated list of L1 candidates (HLTLevel1GTSeed)

How is this seqeunce going to change ?

Then, there are some specific use cases to be identified and updated. A few examples that come to mind are
  - the HLTMuonL1Filter is capable of applying further quality cuts to the L1 muon candidates
  - the L2MuonSeedGenerator uses the L1 muon candidates to seed the L2 reconstruction
  - the HLTEcalRecHitInAllL1RegionsProducer and EgammaHLTCaloTowerProducer use the L1 e/g candidates to select the regions to reconstruct
  - the HLTEgammaL1MatchFilterRegional matches the L1 and HLT e/g candidates (to avoid "volunteers")

Other modules that use the L1 candidates (I did not have a thorough look at them) are
  - CaloTowerCreatorForTauHLT
  - EgammaHLTCaloTowerProducer
  - HLTL1MuonSelector
  - HLTPFJetL1MatchProducer
  - IsolatedPixelTrackCandidateProducer

Note that I'm not saying that you need to migrate all these modules - but you need to provide enough information that the HLT developers can make the changes.


2. identify all other CMSSW modules NOT used at HLT that need to be updated, rewritten, or replaced with new modules

Well, this will need to happen at some point - but I won't include in the "urgent to do list". 
I just mean that once all HLT-related modules have been migrated, the work is not over.


3. identify the dependencies and priorities for the changes

Basically, what needs to be done first, what has to come later, etc.

Probably the earliest deadline is the MGWR in the second half of February: by then we need to be able to unpack the L1 results and filter events based on them.


4. Legacy vs. Stage 2 software

One last thing to consider is how the HLT modules should deal with the Legacy/Stage 1 vs. the Stage 2 software, as in principle, we may need to be able to run both in parallel.
Possible ideas:
  - we could have two copies of th HLT modules (for example via template specialisations) to deal with Legacy vs  Stage 2
  - we could foresee a way to translate the Legacy objects into the Stage 2 counterparts
  - ...

@mulhearn
Copy link
Member Author

Please see also cms-sw#12994 for commit 78550a4 and e8d283c which provide the new HLT enums.

@mulhearn
Copy link
Member Author

Answer to Andrea point 4 is we definitely will provide a module that replaces L1ExtraProducer with something that outputs Legacy/Stage-1 output in the upgrade formats.

@mulhearn
Copy link
Member Author

Q: summarise the changes to the L1 software with respect to Legacy / Stage 1

Emulator Changes

Almost the entire L1T emulation sequence has been replaced for
Stage-2. I'll restrict most of my answer to the changes effecting
consumers of L1 output, but the full details of which modules are
running in the Stage-2 vs Stage-1 vs legacy are contained in the new
era-driven configuration of the L1T-Emulation, which now cleanly
switches between the 3-stages. The top-level config here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/Configuration/python/SimL1Emulator_cff.py

Simply calls the subsytem scripts here (that contain all the details):

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/L1TGlobal/python/simDigis_cff.py
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/L1TCalorimeter/python/simDigis_cff.py
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/L1TMuon/python/simDigis_cff.py

The most important module for HLT will be this one:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/L1TGlobal/python/simGtStage2Digis_cfi.py

The upgrade GT emulator, which you will need to re-emulate in order to
obtain the critical L1 object map, which maps particular L1 objects to
the particular bit that they could contribute to firing.

This new module of course retrieves new inputs, so there are
corresponding updates to the unpacker which are needed. These are
in the new era-driven L1TRawToDigi sequence here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/Configuration/python/L1TRawToDigi_cff.py

For re-emulating L1TGlobal you will need to unpack "caloStage2Digis + gmtStage2Digis".

L1T OBJECT CLASSES

What are the inputs to L1TGlobal?

They are BX vectors (std::vectors that provide per bx iterators, bx=0 corresponding to the trigger bx) of upgrade format L1T objects: EGamma, Tau, Jet, EtSum, Muon:

  BXVector<l1t::EGamma> 
  BXVector<l1t::Tau>  
  BXVector<l1t::Jet> 
  BXVector<l1t::EtSum> 
  BXVector<l1t::Muon> 

Unlike the legacy system, where a separate step was needed to convert
emulator objects (with hardware values) to physical values (e.g. pt in
GeV), the upgrade objects all contain both the digital hardware
values and the values at physical scale. So you can now access the
physical four vectors directly, via the L1Candidate interface which
all of these objects satisfy:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/L1Candidate.h

Note in particular, that the L1Candidate is a reco::LeafCandidate, so in most cases the L1Candidates will be a drop in replacement for the legacy (deprecated) L1Extra Particle class.

If you need object specific quanities (e.g. Muon quality) they are via the object dependent derived classes:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/Jet.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/Muon.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/Sum.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/Tau.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/EGamma.h

The BX vector class is here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1Trigger/interface/BXVector.h

but note that L1 Seeds are provided to you in the same manner as for the legacy system, as std::vectors of edm::Ref to the objects (e.g. std::vector< edm::Refl1t::Jet >.

Again, since this is essentially std::vector<edm::Ref< DERIVED FROM reco::LeafCandidate> > exactly as for legacy system, in most usage cases, the seeds should be a drop in replacement.

GLOBAL TRIGGER OUTPUT

What does this new L1TGlobal emulator produce?

GlobalAlgBlkBxCollection NEW
GlobalExtBlkBxCollection NEW
L1GlobalTriggerObjectMapRecord SAME FORMAT AS FOR LEGACY

Which are all defined here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1TGlobal/interface/GlobalExtBlk.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1TGlobal/interface/GlobalAlgBlk.h
https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/DataFormats/L1GlobalTrigger/interface/L1GlobalTriggerObjectMap.h

The first two objects are the replacement for the legacy
"L1GlobalTriggerReadoutRecord". It is now broken into two parts, one
for L1 algorithms (Alg), and one for triggers based on external (Ext) conditions.

The L1GlobalTriggerObjectMapRecord is the same format as for legacy,
and will continue to provide the (emulated) mapping of which input L1T
objects would fire a particular bit.

CONDITION FORMATS

How are the conditions for the new L1TGlobal set?

At the version of the code you will be give, these are all set via
local XML file, or CSV file for the case of masks and prescale.

There is meanwhile an intense effort to provide updated CondFormats
objects which is nearly converging. We have a new external library
which handles the mapping of XML files to objects, which have
(bitwise) aliases in CondFormats. At some point "soon", the
configuration of L1TGlobal will be via these new CondFormats objects.
You can follow this issue here:

#71

and see the eventual CondFormat objects here:

08b95b5

@mulhearn
Copy link
Member Author

Q: identify all CMSSW modules used at HLT that need to be updated

Scale of Problem

Ooof. I'm not sure I can give a complete answer, since I only know
what currently breaks. I'm not entirely sure what else will break
once these are all fixed, or which modules are suffering in silence.

However, I did do a simple test where I quieted all modules in
standard HLT sequence that complained, after I swapped the legacy L1
seed producer (HLTLevel1GTSeed) with our drop in replacement
(HLTL1TSeed) described in more detail below. For what it is worth
the complete set of modules that complained were:

HLTrigger/Muon/src/HLTMuonL1Filter.cc
RecoEgamma/EgammaHLTProducers/interface/HLTRecHitInAllL1RegionsProducer.h
RecoMuon/L2MuonSeedGenerator/src/L2MuonSeedGenerator.cc
RecoTauTag/HLTProducers/src/CaloTowerCreatorForTauHLT.cc

which a quick scanned looked like its a matter of replacing l1Extra
with the drop in upgrade formats. I didn't look at all yet at this
guy, but just removed him the HLT standard sequence for my test:

HLTLevel1Activity.cc

Comparing with your list, I see that my simple test did not catch
these additional modules, which I'll have to look trhough as well:

HLTEcalRecHitInAllL1RegionsProducer
EgammaHLTCaloTowerProducer
HLTEgammaL1MatchFilterRegional
EgammaHLTCaloTowerProducer
HLTL1MuonSelector
HLTPFJetL1MatchProducer
IsolatedPixelTrackCandidateProducer

HLTL1TSeed

This is the main guy. This is intented as a drop in replacement for
HLTLevel1GTSeed. As of today, v1.0 is available.

The module takes the upgrade inputs and provides the list of L1T seeds
as an std::vector of edm::Refs to upgrade L1Objects. As these upgrade
L1Objects satisfy the same reco::LeafCandidate interace as old L1Extra
particles, this should largely be a drop in replacement for the
downstream code as well.

There is only one major caveat: the v1.0 of this module provides all
of the L1T seeds. An update will re-implement the functionality which
provides the subset of seeds firing a particular list of L1 bits.

L1TGlobalAnalyzer

This module:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-integration-pass3-80x/L1Trigger/L1TGlobal/plugins/L1TGlobalAnalyzer.cc

provides the functionality of the:

https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideL1TriggerL1GtUtils

but is not yet a drop-in replacement.

Note that the new version will not rely on conditions at the time of
use, but will instead use a mapping of trigger bit to strigger name
which is stored in the Event record.

This functionality is not used directly by HLT so hasn't been
prioritized over the HLTLevel1GTSeed module.

@mulhearn
Copy link
Member Author

Q: identify all other CMSSW modules NOT used at HLT that need to be updated, rewritten, or replaced with new modules

I guess the main one here is the replacement for L1GtUtils which has widespread use in CMSSW. Not sure yet what else remains on this list. We'll see which workflows downstream of HLT break once we have that working.

Q: identify the dependencies and priorities for the changes

Punting on this until we talk. I believe we've provided v1.0 of the most important module (HLTL1TSeed) but I don't know how to prioritize the remaining steps.

@mulhearn
Copy link
Member Author

Q: Legacy vs. Stage 2 software

One last thing to consider is how the HLT modules should deal with the Legacy/Stage 1 vs. the Stage 2 software, as in principle, we may need to be able to run both in parallel.
Possible ideas:

  • we could have two copies of th HLT modules (for example via template specialisations) to deal with Legacy vs Stage 2
  • we could foresee a way to translate the Legacy objects into the Stage 2 counterparts

ANSWER: I infinitely prefer option 2. We'll provide a replacement for L1ExtraProducer which converts legacy objects into upgrade format, so that HLT can use the same interface for legacy system after the code updates. There is some complication here in that the legacy objects are broken into different number of collections (e.g. isoTaus and non-iso Taus, compared to upgrade, which simply has Taus.) This has consequences for interpreting the L1TObjectMap...

@rekovic
Copy link

rekovic commented Jan 21, 2016

Need to check functionality of TriggerReportSummary, as suggested by Martin G.

@mulhearn
Copy link
Member Author

The dev branch for L1TGlobal development with the utm library is here (for reference):
https://github.com/cms-l1t-offline/cmssw/tree/l1t-global-jan-dev-CMSSW_8_0_0_pre4

@rekovic
Copy link

rekovic commented Jan 28, 2016

Working note:

I have got the HLTL1TSeed module to access L1GlobalTriggerObjectMapRecord,
and loop over all its Maps, but it turns out it is not enough.

In Legacy L1/HLT interface one does need to be able to get the L1TriggerMenu from the EventSetup.
Once menu is obtained, the algorithm and aliases maps are obtained

   const AlgorithmMap& algorithmMap      = l1GtMenu->gtAlgorithmMap();
   const AlgorithmMap& algorithmAliasMap = l1GtMenu->gtAlgorithmAliasMap();

which are then used to retrieve the GT decision bitNumbers for all seeding conditions.
This bitNumber is then stored as the tokenNumber member variables of each L1GtLogicParser::OperandToken in m_l1AlgoLogicParser. This is the first update of m_l1AlgoLogicParser.

In the method seedsL1TriggerObjectMaps(....) there is a second update of m_l1AlgoLogicParser.
The OperandToken.tokenNumbers are then used to get the decision of the GT decisionWord
and store it in OperandToken.tokenResult for each OperandToken in the vector.
If TRUE then the seeding objects for that OperanToken.tokenName are retrieved
from the L1GlobalTriggerObjectMapRecord.

So, bottom line is, at the moment I need to access the L1TriggerMenu and GT decisonWord (i.e. L1GlobalTriggerReadoutRecord) to be able to continue.

@fwyzard
Copy link

fwyzard commented Jan 28, 2016

@rekovic , don't you have all the necessary information and code available in the branch https://github.com/cms-l1t-offline/cmssw/tree/l1t-global-jan-dev-CMSSW_8_0_0_pre4 ?

@mulhearn
Copy link
Member Author

That relies on utm being installed... and there's some integration work needed, but in principle, yes.

@mulhearn
Copy link
Member Author

The new conditions we'll eventually have in event setup (based on UTM library) are here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-global-jan-dev-CMSSW_8_0_0_pre4/CondFormats/L1TObjects/interface/L1TUtmTriggerMenu.h

You can see this parallels the structure of the old TriggerMenu, but the low-level fields are slightly different. So the GtParser will need a corresponding update too.

@blwiner
Copy link

blwiner commented Jan 28, 2016

Perhaps this is known, but as I mentioned in the L1 meeting on Tuesday.
L1TGlobalUtils pulls the L1 Menu out of the EventSetup and provides methods
for getting bit number by trigger name, or trigger name by bit number,
trigger bit decisions, etc. As an example, these Utils are used in
GtRecordDump.

-Brian

On Thu, Jan 28, 2016 at 6:36 AM, mulhearn notifications@github.com wrote:

The new conditions we'll eventually have in event setup (based on UTM
library) are here:

https://github.com/cms-l1t-offline/cmssw/blob/l1t-global-jan-dev-CMSSW_8_0_0_pre4/CondFormats/L1TObjects/interface/L1TUtmTriggerMenu.h

You can see this parallels the structure of the old TriggerMenu, but the
low-level fields are slightly different. So the GtParser will need a
corresponding update too.


Reply to this email directly or view it on GitHub
#103 (comment)
.

Prof. Brian L. Winer
              Department of Physics
           The Ohio State University
             191 W. Woodruff Ave
              Columbus, OH 43210

               (614) 292-8996 (Office)
               (614) 292-7370 (Lab)

@mulhearn
Copy link
Member Author

Let's chat if possible...

@fwyzard
Copy link

fwyzard commented Mar 16, 2016 via email

@perrotta
Copy link

@mulhearn @rekovic

A segmentation fault was reported when running over the v3 menu, see
https://hypernews.cern.ch/HyperNews/CMS/get/hlt/4197.html

I found out that the problem was there because candEtSumVec was never instantiated in L1TGlobal/src/CorrCondition.cc

Could you please check the fix proposed in
https://hypernews.cern.ch/HyperNews/CMS/get/hlt/4197/1/1/1.html
and either confirm it, or propose a more appropriate one?

Thank you,
Andrea

@Martin-Grunewald
Copy link

@mulhearn @rekovic
And in any case please make PRs for 80X and 81X.

@blwiner
Copy link

blwiner commented Mar 22, 2016

Yes this is the basic fix. We would like to be a little careful about
where that call is made. Can you tell us which release/branch to fix?

-Brian

On Tue, Mar 22, 2016 at 10:26 AM, perrotta notifications@github.com wrote:

@mulhearn https://github.com/mulhearn @rekovic
https://github.com/rekovic

A segmentation fault was reported when running over the v3 menu, see
https://hypernews.cern.ch/HyperNews/CMS/get/hlt/4197.html

I found out that the problem was there because candEtSumVec was never
instantiated in L1TGlobal/src/CorrCondition.cc

Could you please check the fix proposed in
https://hypernews.cern.ch/HyperNews/CMS/get/hlt/4197/1/1/1.html
and either confirm it, or propose a more appropriate one?

Thank you,
Andrea


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#103 (comment)


B. Winer

@rekovic
Copy link

rekovic commented Mar 22, 2016

@blwiner Just make a PR to l1t-integration-CMSSW_8_0_2 and I will propagate the fix to 80X and 81X.

@blwiner
Copy link

blwiner commented Mar 22, 2016

OK thanks. PR will come from Darren

-Brian

On Tue, Mar 22, 2016 at 10:34 AM, rekovic notifications@github.com wrote:

@blwiner https://github.com/blwiner Just make a PR to
l1t-integration-CMSSW_8_0_2 and I will propagate the fix to 80X and 81X.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#103 (comment)


B. Winer

@puigh
Copy link

puigh commented Mar 22, 2016

PR made:
#205

@rekovic can you test?

@Martin-Grunewald
Copy link

For L1T legacy/stage-1, we used the helper class L1GtUtils to access the current prescale set index and also the L1T prescales per algorithm, etc. This class or a pointer to it is usually embedded
in plugins etc., ie, it is not in itself a plugin!
(c.f https://cmssdt.cern.ch/lxr/source/L1Trigger/GlobalTriggerAnalyzer/interface/?v=CMSSW_8_0_3 )
What would be an equivalent stage-2 aware class?

@Martin-Grunewald
Copy link

@rekovic @puigh @mulhearn
Can we get PRs for 80 and 81 with this fix asap?
Thanks!

@Martin-Grunewald
Copy link

@rekovic
Thanks for the PRs!

@rekovic
Copy link

rekovic commented Mar 23, 2016

@Martin-Grunewald Please use PR cms-sw#13809 (80x) and PR cms-sw#13808 (81x).

@rekovic
Copy link

rekovic commented Mar 23, 2016

Checking the full testing recipe
https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideGlobalHLT#Preparing_a_80X_CMSSW_developer
and running 2 steps: L1RePack + HLT, it turns out that additional configuration setting is needed in the HLT step

    process.StableParameters.NumberConditionChips = 3 
    process.StableParameters.OrderConditionChip  = [ 3, 2, 1 ]

to get rid of the error messages from TriggerObjectMap for bits >=192, which should have been fixed with cms-sw#13780

%MSG-e L1GlobalTriggerObjectMapRecord:  HLTL1TSeed:hltL1sETT15BptxAND  23-Mar-2016 01:16:27 CET Run: 256677 Event: 397040194
 ERROR: The requested algorithm name = L1_ETT15_BptxAND does not exist in the trigger menu. Returning zero pointer for getObjectMap.
%MSG
%MSG-w HLTL1TSeed:  HLTL1TSeed:hltL1sETT15BptxAND 23-Mar-2016 01:16:27 CET  Run: 256677 Event: 397040194
 Warning: seed with name L1_ETT15_BptxAND cannot be matched to a L1 algo name in any L1GlobalTriggerObjectMap
%MSG

cms-sw#13780 alone indeed does fixe the above problem in our simple HLT-like test
HLTrigger/HLTfilters/test/runHLTMuonL1TFilter_Stage2.py, but apparently does not do it in full HLT.
I cannot make sense out of it. uGT emulator team ( @puigh and @blwiner ) will be looking at it when
online (US time)

@Martin-Grunewald I am making a PR for it immediately since the above changes were shown to make
the HLT step run smooth (and are also fine for the simple HLT-like test)

@rekovic
Copy link

rekovic commented Mar 23, 2016

Hmmmm....The changes made in the L1Trigger/L1TGlobal/python/StableParameters_cff.py seem not to have an affect when running HLT step 2. (PRs were made from scratch and compiled.) Adding changes directly to the HLT step 2 configuration does have an affect!! Strange!

Adding either the changes suggested in PR cms-sw#13780

process.StableParameters.NumberConditionChips = 1 
process.StableParameters.OrderConditionChip  = [1]
process.StableParameters.PinsOnConditionChip  = 512

or

process.StableParameters.NumberConditionChips = 3 
process.StableParameters.OrderConditionChip  = [ 3, 2, 1 ]

fixes it.

@fwyzard
Copy link

fwyzard commented Mar 23, 2016

The HLT menu includes its own copy of the StableParameters module, and does not read it from the release.

@rekovic
Copy link

rekovic commented Mar 23, 2016

@fwyzard Thanks, I see it now. Its configuration in the HLT menu is obsolete, in which case either of the two config changes to it can/need to be made. To be consistent with cms-sw#13780, we should probably have it updated with

process.StableParameters.NumberConditionChips = 1 
process.StableParameters.OrderConditionChip  = [1]
process.StableParameters.PinsOnConditionChip  = 512

@Martin-Grunewald
Copy link

This is in ConfDB since /dev/CMSSW_8_0_0/HLT/V65 so will eventually make it out!

@rekovic
Copy link

rekovic commented Mar 23, 2016

@Martin-Grunewald OK. Thanks!

@Martin-Grunewald
Copy link

@rekovic @mulhearn @fwyzard

  • There are several PRs awaiting L1 signature for several days now, please go through and sign asap!
  • Any progress on L1GtUtils non-plugin helper class replacement to get prescale set and L1 precales?
  • Could you implement a "L1GlobalDecision" "meta seed (like in the old HLTLevel1GTSeed plugin)" in HLTL1TSeed - which is TRUE if any of the L1 seeds fired?
  • Any progress on a suitable/proper L1_ZeroBias?
  • HLTL1TSeed should probably throw an exception rather than printing to LogError in case of an unknown seed (throwing an exception was the behaviour of HLTLevel1GTSeed).

Thanks and best regards

Martin

@rekovic
Copy link

rekovic commented Mar 31, 2016

@Martin-Grunewald

  • Could you implement a "L1GlobalDecision" "meta seed (like in the old HLTLevel1GTSeed plugin)" in HLTL1TSeed - which is TRUE if any of the L1 seeds fired?
  • Any progress on a suitable/proper L1_ZeroBias?

This development is available in l1t-integration-v19.2.

  • HLTL1TSeed should probably throw an exception rather than printing to LogError in case of an unknown seed (throwing an exception was the behaviour of HLTLevel1GTSeed).

Will make updates for the above three issues and make a PR to CMSSW 81x.

@Martin-Grunewald
Copy link

@rekovic
... and a PR for 80X as well, please! Thanks a lot!

@rekovic
Copy link

rekovic commented Mar 31, 2016

@Martin-Grunewald Ok, yes, and for 80x.

@Martin-Grunewald
Copy link

@rekovic
BTW, L1TGlobalProducer seems to be a legacy EDProducer, which is a performance problem when using mulithreading on the HLT farm. Could you please convert to to a stream or global producer?
Thanks!

@Martin-Grunewald
Copy link

Martin-Grunewald commented Apr 29, 2016

@mulhearn @rekovic
A few long-outstanding issues - please comment, if only on the current status

  • L1TGlobalProducer is legacy, should become a global or stream producer
  • need a replacement for L1GtUtils to get to the prescale set number (common to L1T and HLT) and the L1T algo prescales for a set number and algo name - is L1TGlobalUtil to be used? How?
  • L1T prescale handling - replacement of the csv file with DB/ESmodule/DB?
  • how can one configure L1REPACK to redo/not-redo (depending on use case) L1T prescales & masks?

@mulhearn
Copy link
Member Author

@Martin-Grunewald

  • Haven't updated the L1TGlobalProducer yet or even looked into it. Is this trivial change? Is this performance issue for HLT?
  • The CondFormats update for the recent L1TGlobal PR include new conditions for masks pre-scales and vetos. The team is working on switching the module to use the ESSetup. However, it is being done such that it will have no impact on online HLT configs: if the prescales are not re-emulated, the ES will not even be queried.
  • In works with above.
  • We'll provide a recipe for customizing this when the final version is ready. Do you need something sooner to work with the .csv file?

@mulhearn
Copy link
Member Author

  • I think we need to chat about L1TGtUtils replacement.

@Martin-Grunewald
Copy link

Martin-Grunewald commented Apr 29, 2016

Yes (the second item) L1GtUtils - it is a purely helper class for the user to get to the prescale set number and the actual prescale for some L1T algo. I am trying to make L1TGlobalUtil useable
for that as it contains most of it, but for sure the internals of L1TGlobalUtil would need update
when the source of the prescales is via DB rather than csv file.
For example, the HLTPrescaleProvider uses L1GtUtil to get the combined HLT and L1T prescales corresponding to the HLTL1TSeed module used in HLT.

@mulhearn
Copy link
Member Author

If you can make L1TGlobalUtils provide the info you need in present state, we can handle the update to use the DB. Or if you prefer, you can write up the hooks you would like implemented in L1TGlobalUtil.

Originally, I planned to remove the dependence on conditions for this information, by recording it the Run record. We could still do this, but if so, we'll preserve the L1TGlobalUtil interface. Sound reasonable?

@Martin-Grunewald
Copy link

OK, yes!

@puigh
Copy link

puigh commented Apr 29, 2016

In our development branch, we've already switched to getting prescales via
the event record, as opposed to the csv file:
https://github.com/cms-l1t-offline/cmssw/blob/l1t-global-CMSSW_8_0_5/L1Trigger/L1TGlobal/src/L1TGlobalUtil.cc#L99-L113

On Fri, Apr 29, 2016 at 11:36 AM, Martin Grunewald <notifications@github.com

wrote:

Yes (the second item) L1GtUtils - it is a purely helper class for the user
to get to the prescale set number and the actual prescale for some L1T
algo. I am trying to make L1TGlobalUtil useable
for that as it contains most of it, but for sure the internals of
L1TGlobalUtil would need update
when the source of the prescales is via DB rather than csv file.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#103 (comment)

@Martin-Grunewald
Copy link

Have a look at (and sign) cms-sw#14312
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants