PPS: consume association cuts from DB

[jan-kaspar]

PR description:

This PR updates the default PPS reconstruction configuration to consume the PPSAssociationCuts from DB. A small correction is made to the condition format to ensure that all class fields are correctly initialised.

This PR needs to be considered together with an updated GT. In particular it was tested with 121X_dataRun3_Candidate_2021_10_21_12_37_53.

This is a technical PR, no changes in results are expected.

PR validation:

The plots below compare results before (blue) and after this PR (red dashed)

• dirsim_cmp.pdf: for PPS direct simulation (+ reconstruction), Run2 and Run3
• reco_cmp.pdf: for PPS reconstruction, Run2 data

No differences observed as expected.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35766/26116

• This PR adds an extra 24KB to repository

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-35766/26116/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35766/26116/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[tvami]

Hi @jan-kaspar please note that code-checks have failed, please fix that so we can test this

[tvami]

I mentioned this to Wagner on Monday, could you please also modify your validation script that it includes the ratio inset below each plot? Thanks

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35766/26138

• This PR adds an extra 24KB to repository

[cmsbuild]

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

It involves the following packages:

• CondFormats/PPSObjects (alca)
• RecoPPS/ProtonReconstruction (reconstruction)
• Validation/CTPPS (dqm)

@malbouis, @yuanchao, @pmandrik, @emanueleusai, @ahmad3213, @tvami, @cmsbuild, @jfernan2, @slava77, @jpata, @francescobrivio, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@mmusich, @tocheng, @seemasharmafnal this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[tvami]

Thanks Jan for the code-checks, next step would be to create a new GT that this can be tested, but before that we have the PPS geometry PR and the DD4HEP geometry PR... so this PR will have to wait a bit longer

[jan-kaspar]

I mentioned this to Wagner on Monday, could you please also modify your validation script that it includes the ratio inset below each plot? Thanks

This is obviously very useful, thanks @tvami for motivating to actually do it!

Done: CTPPS/pps-quick-test@eda9142

The updated plots:

• dirsim_cmp.pdf
• reco_cmp.pdf

For my PPS colleauges -- @grzanka @fabferro @wpcarvalho :

• I've only updated the Asymptote version of the plotting scripts, if you wist to update also the ROOT version, please go ahead
• I am going to merge the 12_1_ass_cuts_from_db branch to 12_1 as soon as this PR is merged

[jan-kaspar]

Thanks Jan for the code-checks, next step would be to create a new GT that this can be tested, but before that we have the PPS geometry PR and the DD4HEP geometry PR... so this PR will have to wait a bit longer

Thanks for the info!

Can't you test it with the candidate by @wpcarvalho 121X_dataRun3_Candidate_2021_10_21_12_37_53?

If not, I have personally no difficulties to wait. However, this PR is a sort of fix for the PPS full simulation - I let @mundim and @clemencia to comment about the urgency.

[tvami]

• ass_cuts

These look great, thanks a lot for the quick action!

[tvami]

Can't you test it with the candidate by @wpcarvalho 121X_dataRun3_Candidate_2021_10_21_12_37_53?

Testing we can do, actually you can do and I can trigger the tests :)
Please edit the autoCond file with the relevant candidates. I think 121X_dataRun3_Candidate_2021_10_21_12_37_53 is actually not the relevant candidate, since this is the Offline GT, one should make candidates for the prompt/express and include those. Unless if you have a relval that uses the Offline GT, in that case using '121X_dataRun3_Candidate_2021_10_21_12_37_53` is fine too, but then please tell me what is the wf number.

[tvami]

Testing we can do, actually you can do and I can trigger the tests :)

I forget to add: the merging is what we cant do for a while

[jan-kaspar]

I guess that we would rely on the 136.* workflows. As these are Run2 data, I would have guessed that they use the latest conditions, which are in the offline flavour. I have never checked, I guess you know better.

Technically, Wagner has made also a prompt GT candidate (and I tested that it gives different results from the offline one). But since this doesn't help to get this PR through, I believe we can wait.

[tvami]

• unit test failure is known and has nothing to do with this PR

Just for the record: #35868

[emanueleusai]

+1

[tvami]

Hi @cms-sw/reconstruction-l2 do you have any further comments?

[slava77]

+reconstruction

for #35766 40e98d8

• code changes are in line with the PR description, changes in the reco code are essentially trivial, to remove non-DB ES specifications
• jenkins tests pass and comparisons with the baseline show no differences

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[qliphy]

+1
to fix the IB issues (see discussions in #35914)

[qliphy]

merge

[Dr15Jones]

This is not thread safe and is likely causing the problems seen in the IB.