Add logic in OnlineBeamSpotESProducer

[francescobrivio]

PR description:

This PR adds a real logic in OnlineBeamSpotESProducer to discriminate between the two different BeamSpot workflows.
The logic is based on two parameters timeThreshold (hours) and sigmaZThreshold (cm) which are both configurable.
The logic is the following:

1. If both BeamSpots are older than the timeThreshold (by default 48 hours) retuns a fake BeamSpot
2. If one one of the two BeamSpots is newer than then timeThreshold returns it only if it the fit converged
   (has BeamType == 2) and sigmaZ is larger than the threshold (by default 2 cm)
3. If both BeamSpots are newer than the timeThreshold returns the one that converges and has larger sigmaZ

The checks on sigmaZ, on the creation time and on the convergence of the fit are also applied to the cases where the ESProducer was able to fetch from the DB only one of the two BeamSpot payloads.

The test RecoVertex/BeamSpotProducer/test/test_scalars.py was also updated to use a file available on eos and to comply with the changes made in #34348 (inclusion of the BS online tags in the GTs).

PR validation:

Tested locally using RecoVertex/BeamSpotProducer/test/test_scalars.py.

Backport

A backport to 12_0_X is provided in #35334

FYI @gennai @dzuolo

[francescobrivio]

@cmsbuild please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35333/25380

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @francescobrivio for master.

It involves the following packages:

• RecoVertex/BeamSpotProducer (reconstruction, alca)

@malbouis, @yuanchao, @slava77, @jpata, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @tocheng, @mmusich, @mtosi, @dgulhan 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

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b27811/18737/summary.html
COMMIT: 8b1487c
CMSSW: CMSSW_12_1_X_2021-09-19-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35333/18737/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 40
• DQMHistoTests: Total histograms compared: 3211080
• DQMHistoTests: Total failures: 0
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3211058
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 39 files compared)
• Checked 169 log files, 37 edm output root files, 40 DQM output files
• TriggerResults: no differences found

[tvami]

PR validation:

Tested locally using RecoVertex/BeamSpotProducer/test/test_scalars.py.

@francescobrivio can you maybe post the results of this validation in the description?

[francescobrivio]

PR validation:

Tested locally using RecoVertex/BeamSpotProducer/test/test_scalars.py.

@francescobrivio can you maybe post the results of this validation in the description?

This is meant to be run online (i.e. during data taking), I did the test "offline" changing by hand some of the cuts and conditions (e.g. the timeThreshold and the selection choice) and adding some couts to see that the correct BeamSpot was chosen, nothing more.

[tvami]

+alca

[tvami]

urgent

• backport needed for the next 12_0_X release

[slava77]

@tvami
is this really urgent? The PR description is not suggestive that this is needed for the beam test in October.

[francescobrivio]

@tvami
is this really urgent? The PR description is not suggestive that this is needed for the beam test in October.

The backport (#35334) is urgent for data taking. It is used online to pass the BeamSpot information to HLT. Pilot Beam Test will be the first test with real collisions.

[gennai]

@tvami
is this really urgent? The PR description is not suggestive that this is needed for the beam test in October.
As @francescobrivio already wrote, there us general consensus (HLT, DB and Tracking at least) to have the final code in place fort the beam test in order to check the complete workflow with collisions

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

[mmusich]

There's a conflict now with #35373.
Wouldn't it be better to wait to have that merged in order to make a meaningful test here?

[francescobrivio]

hold

• waiting for fix offline BS swap for express Run3 #35373 to be merged first

[cmsbuild]

Pull request has been put on hold by @francescobrivio
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[tvami]

There's a conflict now with #35373.
Wouldn't it be better to wait to have that merged in order to make a meaningful test here?

ok I can remove the AlCa signature

[mmusich]

@francescobrivio @tvami #35373 needs more work. In particular an update of the Run3 MC Global Tags might be in order. I propose to unhold and merge this that is already signed and I'll rebase the other one once we figure out what to do.

[francescobrivio]

unhold

• given the comment in Add logic in OnlineBeamSpotESProducer #35333 (comment)

[francescobrivio]

+alca

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

[qliphy]

+1