Clean-up of BeamSpot PCL configs

[dzuolo]

PR description:

This PR is intended to make the config files for the BeamSpot PCL workflows more homogenous, namely:

• the default parameters of the BS Fit common to the two workflows are now set to the same values
  • parameters PVFitter.minVertexNTracks, PVFitter.useOnlyFirstPV and PVFitter.minSumPt are now the only ones with different values between the two workflows, as it should be
• removed modification of these defaults (+ some old comments) in the Legacy workflow config
• errorScale set to 1.0 for both workflows (always recomputed offline for ReReco)

PR validation:

Code compiles.
scramv1 b runtests successful.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This is not a backport. No backport needed.

FYI @gennai @lguzzi

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36345/27102

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @dzuolo (Davide Zuolo) for master.

It involves the following packages:

• Calibration/TkAlCaRecoProducers (alca)

@cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks.
@mmusich, @threus, @tocheng 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

[francescobrivio]

@cmsbuild please test

[mmusich]

@francescobrivio wouldn't it be better to test also 1004.0 and 1030.0 which exercise the BeamSpot PCL wf in other conditions outside of the short matrix?

[francescobrivio]

@cmsbuild please abort

[mmusich]

test parameters:

• workflows = 1004.0, 1030.0

[mmusich]

@cmsbuild, please test

[francescobrivio]

test parameters:

* workflows = 1004.0, 1030.0

thanks Marco! as usual, you are faster than anyone else! 😄

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-126377/20957/summary.html
COMMIT: 2111500
CMSSW: CMSSW_12_2_X_2021-12-02-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36345/20957/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-126377/1004.0_RunHI2011+RunHI2011+TIER0EXPHI+ALCAEXPHI+ALCAHARVD1HI+ALCAHARVD2HI+ALCAHARVD3HI+ALCAHARVD5HI
• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-126377/1030.0_RunHLTPhy2017B+RunHLTPhy2017B+TIER0EXPHPBS+ALCASPLITHPBS+ALCAHARVDHPBS+ALCAHARVDHPBSLOWPU

Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 41
• DQMHistoTests: Total histograms compared: 3041955
• DQMHistoTests: Total failures: 5
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 3041927
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 40 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 175 log files, 37 edm output root files, 41 DQM output files
• TriggerResults: no differences found

[francescobrivio]

@mmusich the only real change here is setting the errorScale parameter to 1, which seems reasonable to me since we don't know a priori what the pulls in the PVs will be in Run3. Can be edited in the future when we have data.
Would you agree?

[mmusich]

@francescobrivio

the only real change here is setting the errorScale parameter to 1, which seems reasonable to me since we don't know a priori what the pulls in the PVs will be in Run3. Can be edited in the future when we have data.
Would you agree?

don't we know a priori that the HP wf has a different errorScale compared to the legacy?

[dzuolo]

@francescobrivio

the only real change here is setting the errorScale parameter to 1, which seems reasonable to me since we don't know a priori what the pulls in the PVs will be in Run3. Can be edited in the future when we have data.
Would you agree?

don't we know a priori that the HP wf has a different errorScale compared to the legacy?

Hi Marco! I do not know if this comparison was made in the past. If not, I would propose to set the value to 1 (as measured for the re-reco of the pilot beam data with the legacy wf) and check the values again with the first collision data.

[francescobrivio]

@francescobrivio

the only real change here is setting the errorScale parameter to 1, which seems reasonable to me since we don't know a priori what the pulls in the PVs will be in Run3. Can be edited in the future when we have data.
Would you agree?

don't we know a priori that the HP wf has a different errorScale compared to the legacy?

Hi Marco! I do not know if this comparison was made in the past. If not, I would propose to set the value to 1 (as measured for the re-reco of the pilot beam data with the legacy wf) and check the values again with the first collision data.

@mmusich I also don't remember making this explicit comparison of the errorScale obtained with the two wfs.
In general we do make this measurement prior to every offline re-reco, no matter what wf is used.
So I would agree with @dzuolo on setting it to 1 by default and in the meanwhile (i.e. before data-taking next year) we can make some study on 2018 data to check if there is a systematic difference in the errorScale between HP and Legacy wfs.

[mmusich]

I also don't remember making this explicit comparison of the errorScale obtained with the two wfs.

@francescobrivio I am pretty sure it was done by your colleagues back in Run2. Please check again.

[mmusich]

So I would agree with @dzuolo on setting it to 1 by default and in the meanwhile (i.e. before data-taking next year) we can make some study on 2018 data to check if there is a systematic difference in the errorScale between HP and Legacy wfs.

I'd rather study it in realistic 2021(2) MC.

[francescobrivio]

I also don't remember making this explicit comparison of the errorScale obtained with the two wfs.

@francescobrivio I am pretty sure it was done by your colleagues back in Run2. Please check again.

Ok indeed it was changed by Sara in #23361, following an explicit measurement on 2018 data.
Some checks with MC or real data will need to be re-done next year in any case, so the point now is: is it ok to change them both to 1 or do we want to still keep the old values?

[mmusich]

Some checks with MC or real data will need to be re-done next year in any case, so the point now is: is it ok to change them both to 1 or do we want to still keep the old values?

with the promise to revisit them next year, I guess it's OK for now. Perhaps a comment can be left in configuration about the its significance and how to check it.

[francescobrivio]

+alca

• mainly technical, tests are fine
• errorScale is changed to 1 for both wfs, with the agreement to check again it before next year data taking (see Clean-up of BeamSpot PCL configs #36345 (comment)) @dzuolo please add it to our to do list

[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)

[perrotta]

+1