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

MLPF: first integration of EDProducer and training code #32048

Merged
merged 6 commits into from Jan 4, 2021

Conversation

jpata
Copy link
Contributor

@jpata jpata commented Nov 6, 2020

PR description:

This PR introduces an EDProducer using the MLPF approach, which create reco::PFCandidates, which can be used successfully in downstream CMSSW reconstruction, e.g. AK4 jet clustering.

It's not enabled by default, so nothing should be affected in standard workflows.

Here's an overview of the changes:

  • in RecoParticleFlow/PFProducer, we add MLPFProducer using TensorFlow-CPU, (enabled only with the mlpf modifier, does not affect standard reco outputs even if enabled)
  • the training & sample generation code is located in RecoParticleFlow/PFProducer/test/mlpf_training/
  • in Validation/RecoParticleFlow, we modify the PFAnalysisNtuplizer, which was developed for and is currently used only by MLPF to generate training samples
  • We add a process modifier mlpf and a new workflow 11834.13 where the TF-cpu evaluator of MLPF runs after standard PF and produces a new collection of PFCandidates in the RECO output.

Further work is planned to improve the reconstruction quality by retraining the model, and by integrating the ML-produced PFCandidates further with downstream reconstruction. The goal of this PR is to make the MLPF reconstruction approach easily testable and reviewable by the PF group & CMS more widely.

PR validation:

The latest status slides are here: https://indico.cern.ch/event/963885/#18-update-on-mlpf.

Can be validated using

$ runTheMatrix.py -l 11834.13
...
$ edmDumpEventContent --all 11834.13_*/step3_inRECOSIM.root | grep 'recoPFCandidates_mlpf'
vector<reco::PFCandidate>             "mlpfProducer"              ""                "RECO"         recoPFCandidates_mlpfProducer__RECO

Needs cms-data/RecoParticleFlow-PFProducer#2.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32048/19620

  • This PR adds an extra 104KB to repository

  • Found files with invalid states:

    • RecoParticleFlow/PFProducer/test/mlpf_training/run_tf.sh:

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

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

It involves the following packages:

RecoParticleFlow/PFProducer
Validation/RecoParticleFlow

@perrotta, @andrius-k, @kmaeshima, @ErnestaP, @cmsbuild, @jfernan2, @fioriNTU, @slava77, @jpata can you please review it and eventually sign? Thanks.
@mmarionncern, @cbernet, @lgray, @seemasharmafnal, @hatakeyamak 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

@jfernan2
Copy link
Contributor

jfernan2 commented Nov 6, 2020

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

The tests are being triggered in jenkins.

Copy link
Contributor

@kpedro88 kpedro88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a partial review, focusing on the SONIC part. (Some of the comments on the SONIC producer could also apply to the native producer.)

RecoParticleFlow/PFProducer/plugins/MLPFProducerSonic.cc Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/plugins/MLPFProducerSonic.cc Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/plugins/MLPFProducerSonic.cc Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/plugins/MLPFProducerSonic.cc Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/plugins/MLPFProducerSonic.cc Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/src/MLPFModel.cpp Outdated Show resolved Hide resolved
RecoParticleFlow/PFProducer/test/mlpf_sonic.py Outdated Show resolved Hide resolved
Validation/RecoParticleFlow/plugins/PFAnalysis.cc Outdated Show resolved Hide resolved
Validation/RecoParticleFlow/plugins/PFAnalysis.cc Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

+1
Tested at: 34a7931
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7ae4e2/10554/summary.html
CMSSW: CMSSW_11_2_X_2020-11-06-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2544144
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2544121
  • 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
Copy link
Contributor Author

jpata commented Dec 22, 2020

+reconstruction

@jfernan2
Copy link
Contributor

+1

@kpedro88
Copy link
Contributor

+upgrade

@chayanit
Copy link

+1

@jpata
Copy link
Contributor Author

jpata commented Jan 4, 2021

@cms-sw/operations-l2 kindly pinging

@qliphy
Copy link
Contributor

qliphy commented Jan 4, 2021

+operations

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 4, 2021

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)

@qliphy
Copy link
Contributor

qliphy commented Jan 4, 2021

+1

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

Successfully merging this pull request may close these issues.

None yet