-
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
XML and FedId configuration update for PPS #37147
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37147/28689
|
A new Pull Request was created by @ChrisMisan (Christopher) for master. It involves the following packages:
@malbouis, @yuanchao, @clacaputo, @cmsbuild, @slava77, @jpata, @tvami, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Hi @ChrisMisan has it been considered to move these into a DB object? Having this IOV structure that you have in the config file is exactly what the multi-IOV tags are for. |
test parameters:
|
@cmsbuild , please test |
-1 Failed Tests: RelVals RelVals-INPUT AddOn RelVals
RelVals-INPUT
AddOn Tests
Expand to see more addon errors ... |
Both issues with relvals should be resolved shortly:
@tvami To answer your question, I know this has been discussed, I guess @grzanka and @vavati can better state the reasons for the current configuration. |
Hi @tvami . We can consider moving into a DB objects, I agree it is a cleaner solution. But again we are under time pressure before Run3. Migrating to DB would require some significant changes in our RAW2DATA modules, proper testing etc... |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37147/28692
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37147/28750
|
Pull request #37147 was updated. @malbouis, @yuanchao, @clacaputo, @cmsbuild, @slava77, @jpata, @tvami, @francescobrivio can you please check and sign again. |
@cmsbuild , please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7b2143/22953/summary.html Comparison SummarySummary:
|
+reconstruction
|
+alca
|
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) |
+1 |
PR description:
This PR updates the xml mappings and FedId configuration for PPS run3. Backward compatibility is sustained.
PR validation:
This PR was validated with relvals 136.793, 136.874 (which run PPS reconstruction). New run3 xml's were parsed without issue. We lack the run3 data to conduct more sophisticated tests. For the run2 data, there should be no changes as to the results.
if this PR is a backport please specify the original PR and why you need to backport that PR:
no