Skip to content
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

Remove run2_HLTconditions_2018 modifier from Run3 setting #39084

Merged
merged 2 commits into from
Aug 24, 2022

Conversation

davidwalter2
Copy link
Contributor

@davidwalter2 davidwalter2 commented Aug 16, 2022

PR description:

As discussed in PR 39082, we remove the run2_HLTconditions_2018 from Run3 era setting to avoid using the 2018 specific settings.

PR validation:

validated via cmsDriver.py

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

We intent to do a backport to 12_4

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39084/31593

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Configuration/Eras (operations)

@perrotta, @rappoccio, @cmsbuild, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @missirol, @fabiocos, @Martin-Grunewald this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor

@cmsbuild, please test

@missirol
Copy link
Contributor

Afaics, the net change from this PR will be in nanoAOD wfs using --era Run3; specifically, this PR would remove prefiringweight,l1PreFiringEventWeightTable from triggerObjectTablesTask.

(run2_HLTconditions_2016 | run2_HLTconditions_2017 | run2_HLTconditions_2018).toReplaceWith(triggerObjectTablesTask,_triggerObjectTablesTask_withL1PreFiring)

@cms-sw/xpog-l2 should probably review this.

@mariadalfonso
Copy link
Contributor

assign xpog

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@mariadalfonso,@gouskos,@swertz,@vlimant you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mariadalfonso
Copy link
Contributor

The preferring weights are different based on the year and loaded from here
cms-data/PhysicsTools-PatUtils#2
Does it make sense to have for Run3 the same for Run2 ?

@mariadalfonso
Copy link
Contributor

The preferring weights are different based on the year and loaded from here
cms-data/PhysicsTools-PatUtils#2
Does it make sense to have for Run3 the same for Run2 ?

@JanFSchulte @lathomas

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15f5fa/26862/summary.html
COMMIT: a9ee094
CMSSW: CMSSW_12_5_X_2022-08-16-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39084/26862/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15f5fa/26862/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15f5fa/26862/git-merge-result

RelVals-INPUT

  • 36.0DAS Error

Comparison Summary

There are some workflows for which there are errors in the baseline:
1001.2 step 5
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 19331 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3692500
  • DQMHistoTests: Total failures: 31024
  • DQMHistoTests: Total nulls: 2
  • DQMHistoTests: Total successes: 3661452
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 138.5 ): 0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 3 / 50 workflows

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39084/31662

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #39084 was updated. @perrotta, @rappoccio, @gouskos, @swertz, @vlimant, @fabiocos, @cmsbuild, @qliphy, @mariadalfonso, @davidlange6 can you please check and sign again.

@lathomas
Copy link
Contributor

The preferring weights are different based on the year and loaded from here
cms-data/PhysicsTools-PatUtils#2
Does it make sense to have for Run3 the same for Run2 ?

@JanFSchulte @lathomas

@mariadalfonso
Indeed, the Run 2 weights shouldn't be used for Run 3. Ideally I would keep the variables though (there's a fair chance we still need to apply prefiring weights in Run 3).

@cmsbuild cmsbuild merged commit 695314b into cms-sw:master Aug 24, 2022
@davidwalter2
Copy link
Contributor Author

Now this PR is marked as merged because in the commit chain of PR39119 the same commit was done but reverted again.
So in fact nothing is done here.
On the other hand, we (DQMOffline/Lumi) don't need the changes anymore, so it is okay.
I'm sorry for the confusion.

@mariadalfonso
Copy link
Contributor

Well.
It's clear that extending the era in that way is wrong and the change should not be done in this way.
We need to have the run3 and run2 completely orthogonal, given that we will need two set of maps (as clearly stated by the L1-DPG and also as discussed at the PC with muon_POG).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants