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

Update for DTCalibration offline workflows #20405

Merged
merged 26 commits into from Dec 11, 2017

Conversation

tobias-pook
Copy link
Contributor

Running versions of the DT offline calibration was only available in private forks for some years now. This PR adds a working version again.

The changes include:

New python workflow system using crab3
Fixes to t0 determination
Cleanup and tweaks for segment selection.

serdweg and others added 15 commits July 26, 2017 14:55
This commit includes changes which were done by several past
contributors, but never reached the official repo.
This commit contains a rewrite of the DTcalibration workflow.
Main changes:

* All workflows inherit from one common DTCalibration object
* The CLI is dynamically build and each operations follows the structure
  dtCalibration [workflow] [workflow_mode] [command] using python
  subparsers
* Each worklflow is implemented in a single class.
* Each command needs to implement a function
* prepare_[workflow_mode]_[command]
    which prepares pset files for this command.
* Commonly used commands like submit, check, run are implemented in
  the parent class and may be ovewritten in the child classes if needed.
* Add new crab tools for crab3 integration
* automated proxy creation
* Delete obsolete files in workflows

Documentation about the updated workflow can be found in:
https://twiki.cern.ch/twiki/bin/viewauth/CMS/DTCalibrationTools
Imports of CondCore.DBCommon.ConDBSetup_cfi are deprecated and now
replaced by CondCore.CondDB.CondDB_cfi. The new object does not need the
auhentication mehtod parameter anymore and the connect parameter is
already set and is now set outside of the database objects.
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

A new Pull Request was created by @tobias-pook (Tobias) for master.

It involves the following packages:

CalibMuon/DTCalibration

@ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @mmusich, @tocheng this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2017

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20405/529

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-20405/529/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20405/529/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly (this will soon be required for PRs to be considered)

@arunhep
Copy link
Contributor

arunhep commented Sep 7, 2017

@tobias-pook please implement the code-checker suggestions

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20405/23937/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 26
  • DQMHistoTests: Total histograms compared: 2815041
  • DQMHistoTests: Total failures: 113
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2814757
  • DQMHistoTests: Total skipped: 171
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 10 edm output root files, 26 DQM output files

@tobias-pook
Copy link
Contributor Author

Any updates about the pull request ?

@ptraczyk
Copy link
Contributor

ptraczyk commented Nov 6, 2017

Hi, could we go ahead with this PR? We believe we've addressed all the comments

@lpernie
Copy link
Contributor

lpernie commented Nov 7, 2017

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 7, 2017

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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2)

@ptraczyk
Copy link
Contributor

Hi, we seem to be stuck with this PR again..

@lpernie
Copy link
Contributor

lpernie commented Dec 1, 2017

@davidlange6 Could we merge this PR?
It has been mentioned also in the last ORP

@lpernie
Copy link
Contributor

lpernie commented Dec 4, 2017

Hi @davidlange6 sorry to insist, but what is holding this PR?

@davidlange6
Copy link
Contributor

davidlange6 commented Dec 4, 2017 via email

@lpernie
Copy link
Contributor

lpernie commented Dec 4, 2017

I asked because DT people are waiting for it, and asked me for a time estimation.
I will tell them it needs more time due to the number of files modified.

@davidlange6
Copy link
Contributor

+1
sorry for the slow review

@cmsbuild cmsbuild merged commit ef598b6 into cms-sw:master Dec 11, 2017
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

8 participants