Remove HF/Voronoi subtraction for jets in heavy ions and disable centrality bin producer #18684
Conversation
…rality bin producer
|
@cmsbuild please test @smuzaffar |
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @mandrenguyen for master. It involves the following packages: DQM/Physics @perrotta, @dmitrijus, @cmsbuild, @slava77, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
DQM/Physics/src/CentralityDQM.cc
Outdated
|
|
||
| int hibin = -999; | ||
| if(cbin.isValid()) hibin = *cbin; | ||
| else edm::LogWarning("CentralityDQM") << "invalid collection: centralityBin " << std::endl; |
There was a problem hiding this comment.
what is the reason to pollute the logs with this message by default?
The same question applies to other cases.
If the goal is to keep a reminder where it should be added again, perhaps add these printouts with a compile flag e.g. #ifdef CENTRALITY_WARNING
cmsbuild
added
comparison-pending
dqm-pending
and removed
comparison-available
dqm-approved
tests-approved
labels
DQM/Physics/src/CentralityDQM.cc
Outdated
| @@ -143,7 +143,7 @@ void CentralityDQM::analyze(const edm::Event& iEvent, | |||
| if(cent.isValid()){ | |||
| int hibin = -999; | |||
| if(cbin.isValid()) hibin = *cbin; | |||
| else edm::LogWarning("CentralityDQM") << "invalid collection: centralityBin " << std::endl; | |||
| //else edm::LogWarning("CentralityDQM") << "invalid collection: centralityBin " << std::endl; | |||
There was a problem hiding this comment.
is this commented out code needed?
Please remove or add comments inline in the code why the commented out block is relevant
... BTW, these LogWarnings was just the first comment/topic from reco review, I'm not done yet.
|
what is the relationship between the centrality producer and phase-1 setup? |
|
@slava77 The HI reco currently only requires HI-specific GTs for the Vs jets (which are being removed) and for the centralityBinProducer, which can anyway be run post-reco. All the centralityBinProducer does is take the output of hiCentrality and flatten into a percentile. With it disactivated we can just run the HI reco with pp GTs, which is sufficient for the ongoing development work. I prefer to defer an HI specific Phase I GT until it's actually needed. There's no Phase I specific issue with hiCentrality. |
| @@ -795,11 +795,15 @@ void JetAnalyzer_HeavyIons::analyze(const edm::Event& mEvent, const edm::EventSe | |||
| edm::Handle<int> cbin; | |||
| mEvent.getByToken(centralityBinToken, cbin); | |||
|
|
|||
| if (!cent.isValid()) return; | |||
| if (!cent.isValid()){ | |||
| edm::LogError("JetAnalyzer_HeavyIons") << "invalid collection: centrality " << std::endl; | |||
There was a problem hiding this comment.
If this is a real problem, better throw an exception (or let the framework throw it).
Since a while now, it can take a long time until a LogError is noticed.
|
If Voronoi algo is gone for good, please remove bpmpd*f and also cleanup RecoHI/HiJetAlgos/plugins/BuildFile.xml from CGAL and fortran77 needs. |
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-available
tests-approved
labels
|
Pull request #18684 was updated. @perrotta, @dmitrijus, @cmsbuild, @slava77, @vanbesien, @davidlange6 can you please check and sign again. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1
@davidlange6 |
Clean-up of the heavy-ion reco sequence to set the stage for developments for the 2018 Run.
Without these two items the phase 1 version of the heavy-ion tracking #18646 can run directly from the cmsDriver without removing sequences "by-hand"
These changes required some cleanup of DQM/Validation sequences (removing Vs jets and adding protection if centrality bins are not produced).
The PR was tested on wfs 145 and 140.53 in addition to runTheMarix.py -s
@abaty @kurtejung @cfmcginn @mverwe @jmartinb