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

Add PandoraTranslator to RecoParticleFlow #7961

Merged
merged 7 commits into from Feb 26, 2015

Conversation

kpedro88
Copy link
Contributor

This pull request adds PandoraTranslator, previously a separate repo, to CMSSW in the RecoParticleFlow area. This enables the use of PandoraPFA for HGCal reconstruction, including the subsequent creation of CMS PF objects for easy use in analysis.

The customization function to use PandoraTranslator is currently invoked as follows:
from RecoParticleFlow.PandoraTranslator.customizeHGCalPandora_cff import cust_2023HGCalPandoraMuon
process = cust_2023HGCalPandoraMuon(process)

This PR uses the "slow" recipe without fast Pandora algorithms optimized for high pileup. In addition, it will not compile without an edit to the xml for the external package:
CMSSW_6_2_X_SLHC_2015-02-26-0200/config/toolbox/slc6_amd64_gcc472/tools/selected/pandora.xml
remove:

When the Pandora external package is updated to include the latest version with the fast algorithms, another PR will be issued to restore the use of those algorithms in PandoraTranslator.

Attn: @vandreev11 @pfs @lgray @sethzenz

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_6_2_X_SLHC.

Add PandoraTranslator to RecoParticleFlow

It involves the following packages:

RecoParticleFlow/PandoraTranslator

The following packages do not have a category, yet:

RecoParticleFlow/PandoraTranslator

@cmsbuild, @nclopezo can you please review it and eventually sign? Thanks.
@mmarionncern, @bachtis, @lgray this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@fratnikov, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

-1
Tested at: e0c302f
I found an error when building:

Leaving library rule at FWCore/Version
>> Leaving Package FWCore/Version
>> Package FWCore/Version built
>> Subsystem FWCore built
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc472/external/gcc/4.7.2-cms/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.7.2/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lPandoraMonitoring
collect2: error: ld returned 1 exit status
gmake: **\* [tmp/slc6_amd64_gcc472/src/RecoParticleFlow/PandoraTranslator/src/RecoParticleFlowPandoraTranslator/libRecoParticleFlowPandoraTranslator.so] Error 1
Leaving library rule at RecoParticleFlow/PandoraTranslator
>> Building edm plugin tmp/slc6_amd64_gcc472/src/RecoParticleFlow/PandoraTranslator/plugins/HGCalPandoraTranslatorPlugins/libHGCalPandoraTranslatorPlugins.so
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc472/external/gcc/4.7.2-cms/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.7.2/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lRecoParticleFlowPandoraTranslator
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc472/external/gcc/4.7.2-cms/bin/../lib/gcc/x86_64-unknown-linux-gnu/4.7.2/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lPandoraMonitoring


you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7961/2950/summary.html

@sethzenz
Copy link
Contributor

Note that the automated test's compilation failure above is expected unless the xml for the external package is updated; details given in @kpedro88's original comment.

@kpedro88
Copy link
Contributor Author

IB CMSSW_6_2_X_SLHC_2015-02-26-1400 includes the updated Pandora external, so I have updated this PR to use the fast algorithms. Everything checks out in my quick tests.

Note: it is also important to merge #7954 in the next IB to quiet down the logs.

@cmsbuild
Copy link
Contributor

@kpedro88
Copy link
Contributor Author

Mark: yes, this commit now has everything.

@fratnikov
Copy link

@kpedro88 Is Pandora enabled already? In which configuration file Pandora is this done?
How to select which PF algorithm to run in the cmssw job?

@lgray
Copy link
Contributor

lgray commented Feb 26, 2015

Hi @fratnikov, in the PR description there are instructions for switching on pandora for the hgc using a standard customize function.

@fratnikov
Copy link

merge

cmsbuild added a commit that referenced this pull request Feb 26, 2015
Add PandoraTranslator to RecoParticleFlow
@cmsbuild cmsbuild merged commit 2ec73d3 into cms-sw:CMSSW_6_2_X_SLHC Feb 26, 2015
@davidlange6
Copy link
Contributor

@lgray -
that is to say, one should replace

from SLHCUpgradeSimulations.Configuration.combinedCustoms import cust_2023HGCalMuon process = cust_2023HGCalMuon(process)

with

from RecoParticleFlow.PandoraTranslator.customizeHGCalPandora_cff import cust_2023HGCalPandora_common
process = cust_2023HGCalPandora_common(process)

? or is there more? My job is up to 40 minutes on the first event.../store/relval/CMSSW_6_2_0_SLHC23/RelValQCD_Pt_80_120_14TeV/GEN-SIM-DIGI-RAW/PU_PH2_1K_FB_V6_HGCalV5PU140-v1/00000/02A9D980-189C
-E411-B8A0-0025905964A2.root

note that the RecoParticleFlow/PandoraTranslator/test/test_customization.py will have the wrong tracking configuration as the pileup is not specified (but the tracking will be slower than it should be in that case)

@sethzenz
Copy link
Contributor

@davidlange6 Yes, that replacement is correct. I don't see an obvious reason why your tests are taking 40 minutes. I'll email you separately in a moment about the tracking config in the customization file and perhaps we can discuss the details of the test setup.

@mark-grimes
Copy link

My first event of

/RelValQCDForPF_14TeV/CMSSW_6_2_0_SLHC23_patch2-PU_PH2_1K_FB_V6_HGCalV5PU140-v1/GEN-SIM-DIGI-RAW

took 88 minutes. I replaced the customisation as @davidlange6 says, I get a configuration error otherwise. I hadn't noticed the tracking would be incorrect.

@mark-grimes
Copy link

For the record, here's my cmsDriver command:

xrdcp root://eoscms//eos/cms/store/relval/CMSSW_6_2_0_SLHC23_patch2/RelValQCDForPF_14TeV/GEN-SIM-DIGI-RAW/PU_PH2_1K_FB_V6_HGCalV5PU140-v1/00000/0405B7D0-34AE-E411-A941-0025905A6092.root ./step2.root
cmsDriver.py step3 --conditions PH2_1K_FB_V6::All --pileup_input das:/RelValMinBias_TuneZ2star_14TeV/CMSSW_6_2_0_SLHC20_patch1-DES23_62_V1_refHGCALV5-v1/GEN-SIM -n 100 --eventcontent RECOSIM -s RAW2DIGI,L1Reco,RECO --datatier GEN-SIM-RECO --pileup AVE_140_BX_25ns --customise RecoParticleFlow/PandoraTranslator/customizeHGCalPandora_cff.cust_2023HGCalPandoraMuon --geometry Extended2023HGCalMuon,Extended2023HGCalMuonReco --magField 38T_PostLS1 --python RecoFullPUHGCAL_Extended2023HGCalMuonPU.py --no_exec --filein file:step2.root --fileout file:step3.root

Also should say I'm on 70 minutes already for the second event. I'm on a standard lxplus node and whenever I check top I have 100% CPU, so don't think it's getting starved for resources.

@lgray
Copy link
Contributor

lgray commented Feb 27, 2015

Hi Guys, It sounds like the some of the optimizations aren't turned on. I'll take a look.

@lgray
Copy link
Contributor

lgray commented Feb 27, 2015

@kpedro88 @mark-grimes @davidlange6 Indeed:
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/RecoParticleFlow/PandoraTranslator/python/runPandora_cfi.py#L12

In the HEAD of CMSSW_6_2_X_SLHC is still using the slow versions of the algorithms. It is no mystery why you see such poor performance.

@mark-grimes
Copy link

So just switch to the commented out one on line 13?

@lgray
Copy link
Contributor

lgray commented Feb 27, 2015

Oui. Could you submit the corresponding PR too?

@mark-grimes
Copy link

Sure.

@mark-grimes
Copy link

#7985 submitted to switch the XML configuration file to the faster one.
@davidlange6, I was going to also fix the tracking problem you mentioned, but I think it's okay. The pandora customisation calls phase2TkCustomsBE5DPixel10D.customise, which checks the pileup and quits if it's not defined. Or have I misunderstood the problem?

EDIT - sorry, just seen you were only talking about the test_customization.py

@davidlange6
Copy link
Contributor

@mark-grimes - thats right, the customize function is ok. However, I think the check on pileup I (I think) implemented doesn't always work as process.mix is there regardless. It might be better if it also caught the second if statement on hasattr(process.mix,'input'), but not sure..

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

7 participants