search

cms-l1t-offline / cmssw

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

Update jet collection used by HPS taus #1212

Closed EmyrClement closed 9 months ago

EmyrClement commented 9 months ago

PR description:

A commit that was missed for #1210

PR validation:

Ran L1 sequence, see new collection in output root file.

EmyrClement commented 9 months ago

We should also probably add a requirement that the jet collection is only consumed if fUseJets_ is true:

https://github.com/cms-l1t-offline/cmssw/blob/85d58ce03560972fcebbb69b34f23410332f67fb/L1Trigger/Phase2L1ParticleFlow/plugins/L1HPSPFTauProducer.cxx#L64

Maybe that's a comment we can add to the HPS tau PR to master?

triggerDoctor commented 9 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -j 8
triggerDoctor commented 9 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks! Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 code-checks && scram b -k -j 8 code-checks
I found no issues with the headers! Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 check-headers
triggerDoctor commented 9 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found 1 files that did not meet formatting requirements:

Please run scram b code-format to auto-apply code formatting Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 code-format-all
aloeliger commented 9 months ago

We should also probably add a requirement that the jet collection is only consumed if fUseJets_ is true:

https://github.com/cms-l1t-offline/cmssw/blob/85d58ce03560972fcebbb69b34f23410332f67fb/L1Trigger/Phase2L1ParticleFlow/plugins/L1HPSPFTauProducer.cxx#L64

Maybe that's a comment we can add to the HPS tau PR to master?

I think it needs to be consumed regardless to be passed to this function:

https://github.com/cms-l1t-offline/cmssw/blob/85d58ce03560972fcebbb69b34f23410332f67fb/L1Trigger/Phase2L1ParticleFlow/plugins/L1HPSPFTauProducer.cxx#L139

the result of fUseJets is passed here too, so without reading too much further I would guess it is fine flagging using jets or not for it's own behavior, but attempting to prevent the read overall may be more trouble than it is worth.

aloeliger commented 9 months ago

@EmyrClement I've tested that this fix works. I am going to merge this for the v2 tag. Please ASAP open a PR for #1204, #1210, and #1212 to CMSSW (as one PR is probably easiest, but however you can make work).

EmyrClement commented 9 months ago

@aloeliger I won't open a PR in master for this, as it's already fixed in #43848, specifically in the corresponding fillDescriptions, which replaced the python config modified here.