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

PPS: proton reconstruction, algorithm update #26335

Merged
merged 9 commits into from Apr 12, 2019

Conversation

jan-kaspar
Copy link
Contributor

@jan-kaspar jan-kaspar commented Apr 3, 2019

PR description:

This PR updates the PPS proton reconstruction code with minimal changes to the configuration. It contains several bugfixes and small improvements of the existing code. Mainly, it brings new algorithms:

  • Local tracks (input to proton reconstruction) with large angles are discarded (they cannot be coming from IP).
  • Local tracks in different RPs of the same arm are associated according to optics-driven correlations. On one side, this prevents spurious track combinations (backround). On the other, it allows to perform proton reconstruction even if multiple tracks are present.
  • Local tracks from timing RPs are associated with tracks from tracking RPs. This allows to append timing information to the reconstructed-proton candidates.

This PR is essential for the UL re-reco.

PR validation:

Below a comparison test of re-running the full PPS reco chain on
/store/data/Run2016B/DoubleEG/RAW/v2/000/275/371/00000/FE9F0F13-9436-E611-8F39-02163E012B47.root. Red histograms are obtained with #26305, the blue ones with this PR. The differences are very small. The seemingly big difference in the xi plots is due to different binnings. In the other histograms (where the same binning is used), blue curves are never above the red ones. This is expected since in 2016, the (strip) detectors could not resolve more than 1 local track. Thus the only effect of this PR is the removal of spurious/doubtful local-track combinations.

cmp_45

cmp_56

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26335/9048

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

A new Pull Request was created by @jan-kaspar for master.

It involves the following packages:

CalibPPS/ESProducers
DataFormats/ProtonReco
RecoCTPPS/ProtonReconstruction
Validation/CTPPS

@perrotta, @andrius-k, @kmaeshima, @schneiml, @tocheng, @cmsbuild, @franzoni, @jfernan2, @fioriNTU, @slava77, @pohsun can you please review it and eventually sign? Thanks.
@forthommel, @rovere, @mmusich, @tocheng this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Apr 3, 2019

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33931/console Started: 2019/04/03 16:00

@andrius-k
Copy link

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

-1

Tested at: baa62b2

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

I found follow errors while testing this PR

Failed tests: RelVals AddOn

  • RelVals:

When I ran the RelVals I found an error in the following workflows:
4.53 step3

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step3_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log

136.7611 step2
runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log

136.8311 step2
runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log

136.85 step3
runTheMatrix-results/136.85_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM/step3_RunEGamma2018A+RunEGamma2018A+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Prompt_L1TEgDQM+HARVEST2018_L1TEgDQM.log

  • AddOn:

I found errors in the following addon tests:

cmsDriver.py RelVal -s HLT:PRef,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_PRef --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_PRef_DATA.root --fileout file:RelVal_Raw_PRef_DATA_HLT_RECO.root : FAILED - time: date Wed Apr 3 18:30:05 2019-date Wed Apr 3 18:21:58 2019 s - exit: 16640
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018_pp_on_AA --processName=HLTRECO --filein file:RelVal_Raw_HIon_DATA.root --fileout file:RelVal_Raw_HIon_DATA_HLT_RECO.root : FAILED - time: date Wed Apr 3 18:32:44 2019-date Wed Apr 3 18:21:59 2019 s - exit: 16640
cmsDriver.py RelVal -s HLT:PIon,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_PIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_PIon_DATA.root --fileout file:RelVal_Raw_PIon_DATA_HLT_RECO.root : FAILED - time: date Wed Apr 3 18:37:02 2019-date Wed Apr 3 18:28:17 2019 s - exit: 16640
cmsDriver.py RelVal -s HLT:GRun,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_GRun --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_GRun_DATA.root --fileout file:RelVal_Raw_GRun_DATA_HLT_RECO.root : FAILED - time: date Wed Apr 3 18:47:13 2019-date Wed Apr 3 18:36:33 2019 s - exit: 16640

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2019

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 59 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3140495
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3140295
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@andrius-k
Copy link

+1

@slava77
Copy link
Contributor

slava77 commented Apr 11, 2019

+1

for #26335 dfa0859

  • code changes are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with the baseline show differences only related to PPS protons

My observations of the differences are in line with #26335 (comment)

@pohsun
Copy link

pohsun commented Apr 11, 2019

+1

@fabiocos
Copy link
Contributor

@santocch the analysis part is restricted to PhysicsTools/PatAlgos, could you please have a look? This PR is needed for pre4

1 similar comment
@fabiocos
Copy link
Contributor

@santocch the analysis part is restricted to PhysicsTools/PatAlgos, could you please have a look? This PR is needed for pre4

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit c4df47f into cms-sw:master Apr 12, 2019
@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

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 be automatically merged.

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