remove vertex type from PFCandidate, iteration 2

[jpata]

PR description:

Continuation of #31298 from @veelken, to be able to change the PR.

attn @hatakeyamak @bendavid

[cmsbuild]

The code-checks are being triggered in jenkins.

[jpata]

Need to implement the following: #31298 (comment)

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31456/18354

• This PR adds an extra 12KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File DataFormats/PatCandidates/src/classes_def_objects.xml modified in PR(s): Add track-based isolation, recHit energy sum to IsolatedTrack class #31399

[cmsbuild]

A new Pull Request was created by @jpata (Joosep Pata) for master.

It involves the following packages:

DataFormats/ParticleFlowCandidate
DataFormats/PatCandidates
RecoParticleFlow/PFProducer

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @gouskos, @cbernet, @rovere, @lgray, @hatakeyamak, @gpetruc, @peruzzim, @seemasharmafnal this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[jpata]

I tried something like the following to correctly set the reco::PFCandidate -> reco::LeafCandidate::m_state.vertex_ data member from DataFormats/ParticleFlowCandidate/src/classes_def.xml for old versions of the class, but this doesn't seem to work, as I can't override a parent class member.

  <ioread sourceClass = "reco::PFCandidate" version="[-19]" targetClass="reco::PFCandidate" source="PFVertexType vertexType_" target="m_state.vertex_">
    <![CDATA[m_state.vertex_= setVertexLegacy(onfile.vertexType_)]]>
  </ioread>

We need to change the behaviour of PFCandidate::vertex() for the old versions, but it seems like with ioread rules we can only change data members.

Any suggestions @slava77 et al?

[slava77]

There is a public setVertex method

The set in setVertexLegacy implies an action, not just a return value.
So, setVertex(vertexLegacy(onfile.vertexType_)) ?

[jpata]

Thanks Slava! Just a note to myself, the following seems to be going in the right direction

+  <ioread sourceClass = "reco::PFCandidate" version="[-19]" targetClass="reco::PFCandidate" source="reco::PFCandidate::PFVertexType vertexType_" target="">
+    <![CDATA[newObj->setVertex(newObj->vertexLegacy(onfile.vertexType_));]]>

but currently crashes:

----- Begin Fatal Exception 15-Sep-2020 16:21:01 EEST-----------------------
An exception of category 'FileReadError' occurred while
   [0] Rethrowing an exception that happened on a different thread.
   [1] Reading branch recoPFCandidates_particleFlow__RECO.
Exception Message:
Ref Attempt to construct a Ref with ProductID 2:3076
but with a null pointer to a product getter.
The product getter pointer passed to the constructor must refer
to a real getter, such as an EventPrincipal.
----- End Fatal Exception -------------------------------------------------

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31456/18414

• This PR adds an extra 24KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File DataFormats/PatCandidates/src/classes_def_objects.xml modified in PR(s): Add track-based isolation, recHit energy sum to IsolatedTrack class #31399

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format:
  https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31456/18414/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31456/18414/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31456/18544

• This PR adds an extra 100KB to repository

[cmsbuild]

Pull request #31456 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

[veelken]

@jpata The code looks good to me. Thank you very much for the time that you invested to make this work!

[jpata]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-09-22-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/9510/console Started: 2020/09/23 08:27

[cmsbuild]

+1
Tested at: d568fe4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9798c/9510/summary.html
CMSSW: CMSSW_11_2_X_2020-09-22-2300
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9798c/9510/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 11 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 2540471
• DQMHistoTests: Total failures: 13
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2540436
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 149 log files, 22 edm output root files, 35 DQM output files

[jpata]

+reconstruction

• fixes issue Review of reco::PFCandidate::vertex() function #30895, the vertex is now embedded directly in the PFCandidate at creation time. The new version of the class always has PFCandidate::vertexType_ = kCandVertex, while backwards-compability is retained with old AOD files using the PFCandidate::vertexLegacy(vertexType) function.
• This PR also fixes a bug for electrons, where the KF track was used instead of the GSF track.
• minor reco changes to tau ID are understood to be caused by the electron issue above.

[cmsbuild]

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1