L1Nano addition #40663
L1Nano addition #40663
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40663/34001
|
|
A new Pull Request was created by @Sam-Harper (Sam Harper) for master. It involves the following packages:
@cmsbuild, @swertz, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
assign l1 |
|
New categories assigned: l1 @epalencia,@rekovic,@cecilecaillol you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
Hello, Thanks ! |
| @@ -325,6 +326,57 @@ class SimpleFlatTableProducer : public SimpleFlatTableProducerBase<T, edm::View< | |||
| std::vector<std::unique_ptr<ExtVariable<T>>> extvars_; | |||
| }; | |||
|
|
|||
| template <typename T> | |||
| class BXVectorSimpleFlatTableProducer : public SimpleFlatTableProducerBase<T, BXVector<T>> { | |||
There was a problem hiding this comment.
please add a fillDescription to this new module,
There was a problem hiding this comment.
of course, I didnt do it as it hadnt been done for the others but thats because I was basing of 12_4_X, I see its all done now in the main branch so I can easily make that change.
|
enable nano |
@lathomas I made a size estimate from a
The nano file is here: |
|
thanks @artlbv So on size, it should also be considered that right now it adds a lot of stuff which may not be necessary for many users. And Laurent was suggesting some Pt cuts. So there could be two formats, a minimal one for users and an extended one for experts. Also re the dicussion about having a the bx embedded in the table. I will do some size studies on this but a possible solution would also that the minimal format only has bx=0 and the table adapted so that if minBX=maxBX then the bx variable is auto dropped as its implicit. |
|
Thanks ! So it is huge. Clearly not affordable for central NANO. |
Indeed, this should help. You probably don't need full-precision floats (and ints) everywhere? Note that also the pt is stored with full precision by default, from https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/common_cff.py#L40 |
|
Hi, so I made some quick checks on the size/BX cuts:
Reference:
To summarise: after requiring BX0 and int16 for HW values the total L1 object size goes down from 0.6 to 0.2 kb/evtSurely this dataset is not the most representative one, but this is merely to show there is potential for compression. Summary tables copy-pasted below: Original (as in PR)
Compressed (
|
| collection | kind | vars | items/evt | kb/evt | b/item | plot | % | ascending cumulative % | descending cumulative % |
|---|---|---|---|---|---|---|---|---|---|
| L1Tau | collection | 17 | 10.20 | 0.061 | 6.2 |  |
23.1% | 23.1% | 100.0% |
| L1Mu | collection | 25 | 2.41 | 0.050 | 21.1 | |
18.6% | 41.8% | 76.9% |
| L1EtSum | collection | 7 | 17.00 | 0.033 | 2.0 | |
12.5% | 54.3% | 58.2% |
| L1EG | collection | 18 | 4.25 | 0.033 | 7.9 | |
12.3% | 66.6% | 45.7% |
| L1Jet | collection | 19 | 3.08 | 0.029 | 9.6 | |
10.9% | 77.5% | 33.4% |
| variables = cms.PSet(l1ObjVars, | ||
| towerIEta = Var("towerIEta()",int,doc="tower ieta"), | ||
| towerIPhi = Var("towerIPhi()",int,doc="tower iphi"), | ||
| rawEt = Var("rawEt()",int,doc="raw et"), |
There was a problem hiding this comment.
towerIEta, towerIPhi, and rawEt are repeated for l1EG's, l1Tau's and l1Jet's. Could you maybe save a few lines by just making those a PSet too?
There was a problem hiding this comment.
thanks for the comment, we've now reworked this file a little and will consider if it still makes sense to do this which it probably does as it means we only have to write the doc string oncey
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dec96d/31640/summary.html Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
|
I think the buildbot maintainers might need to tweak the nano testing step report for the inclusion of workflow Other than the fact that the report for it doesn't seem to exist here this seems fine? |
|
The workflow ran fine, which is all we wanted to see: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dec96d/31640/runTheMatrixNANO-results/2500.5111_NANOdata124Xrun3/ Indeed since there is no reference, the comparison fails. Once this is merged, we will have a reference for the next time we make any changes to this. |
|
+xpog L1 content additions for "special prompt Nano" and for L1-specific private Nano. Will you prepare a backport to 13_0 (and perhaps edit the title of this PR, which has become misleading)? |
|
+l1 Thanks for these development @Sam-Harper! We're excited to see L1 contents at NanoAOD, and I've been excited and ready to sign on this for a while. |
|
Hello, I'd like to echo @aloeliger thanks to @Sam-Harper . Thanks also to all people involved. This will be really useful for us. |
|
Hi guys, |
|
+pdmv L1, welcome to Nano |
|
+Upgrade |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
So I was helping some folks with some Eg HLT studies and they mentioned yesterday having the L1 objects in Nano would be useful and I agreed and noticed it didnt seem to exist already
So I quickly made an implementation to see if folks like it and want to put it in to the release. We can discuss here if we think this is useful. The L1 DPG obviously should be on board here (@lathomas FYI)
For the Nano the trick is the BXVectors, I made a new class for this which you can restrict the bx. Also it will write the bx into the table as well. I didnt add singleton and external variable support but that could be easily added if we want to.
I wasnt thinking of having this as part of the standard nano, simply as an option for those wanted to use it for studies.
I'm happy to take suggestions. Also of note, I simply put all the L1 variables in, of course these are not always filled (I think to fill them you need to re-run the emulator not simply unpack), however they are zero as a result. Also probably could adjust the sizes used to store the values.
We could have two modes for this, one which has all L1 objects and info and one which just has a subset if folks are interested.
The only L1 object I'm aware missing is the MuonShowers and that is simply as I didnt have the format to test.
PR validation:
I successfully ran this on 12_4_10 making some nice L1 nano which looked as expected (ie basically the L1 objects in nano). Without activating the nanoL1TrigObjCustomize, no changes are expected.