-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Use ECAL ratio timing algorithm for Run 1 and Run 2, and CC timing algorithm for Run 3 and beyond #42928
Use ECAL ratio timing algorithm for Run 1 and Run 2, and CC timing algorithm for Run 3 and beyond #42928
Conversation
…3 and use ratio timing for Run1 and Run2.
type ecal |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42928/37070
|
A new Pull Request was created by @thomreis (Thomas Reis) for master. It involves the following packages:
@rappoccio, @fabiocos, @davidlange6, @jfernan2, @cmsbuild, @antoniovilela, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
-1 Failed Tests: UnitTests RelVals RelVals-INPUT AddOn Unit TestsI found 3 errors in the following unit tests: ---> test TestDQMOnlineClient-ecal_dqm_sourceclient had ERRORS ---> test TestDQMOnlineClient-visualization had ERRORS ---> test TestDQMOnlineClient-visualization_secondInstance had ERRORS RelVals
Expand to see more relval errors ...RelVals-INPUT
AddOn Tests
Expand to see more addon errors ... |
As mentioned in the description exceptions of the |
test parameters: |
@cmsbuild, please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a minor suggestion, you can actually drop cms type specifications for extra safety.
RecoLocalCalo/EcalRecProducers/python/ecalMultiFitUncalibRecHit_cfi.py
Outdated
Show resolved
Hide resolved
-1 Failed Tests: Build BuildI found compilation error when building: >> Done generating edm plugin poisoned information gmake[1]: *** [config/SCRAM/GMake/Makefile.rules:1937: CompilePython] Error 1 >> Plugins of all types refreshed. gmake[1]: Target 'PostBuild' not remade because of errors. gmake[1]: Leaving directory '/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-10-06-1100' gmake: *** [config/SCRAM/GMake/Makefile.rules:1825: src] Error 2 gmake: Target 'all' not remade because of errors. gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2 + eval scram build outputlog '&&' '(python3' /data/cmsbld/jenkins/workspace/ib-run-pr-tests/cms-bot/buildLogAnalyzer.py --logDir /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_13_3_X_2023-10-06-1100/tmp/el8_amd64_gcc11/cache/log/src '||' 'true)' ++ scram build outputlog >> Entering Package Configuration/Eras |
please test
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cefc69/35084/summary.html Comparison SummarySummary:
|
We expect changes in the ECAL timing with this PR since before many workflows were using the default CC timing algorithm but with the ratio timing calibrations in the GTs. This should be fixed now with this PR. |
urgent
|
@thomreis Just checking |
Yes it is possible that the changed timing has small effects on other distributions as well. Depending, e.g. how the out of time flag is set. |
+reconstruction |
+1 |
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. |
PR description:
This PR sets the default ECAL timing algorithm to the ratio method. This is the default timing algorithm for Run 1 and Run 2.
For Run 3 and Phase 2 a new modifier
run3_ecal
is used to change the timing algorithm to CC timing and use different records with the label 'CC' for the timing calibrations and timing offset constants.Note that this behaviour is different than what was discussed in the meeting on the 27th Sept. when the plan was made to make the CC timing the default and use eras to modify Run 1 and Run 2 configurations to use the ratio method. Since Run 1 configurations could not be modified using eras we decided to reverse the behaviour as in the description above.
PR validation:
The PR passes Run 1 and Run2 WFs in the limited matrix tests but currently fails all Run 3 and Phase 2 WFs because the consumed records with the 'CC' label are not yet in the GTs.