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

Correcting path names #20213

Merged
merged 9 commits into from
Sep 6, 2017
Merged

Conversation

mohsinwaseem
Copy link
Contributor

Path names ending in v* did not seem to work and thus code was returning empty histograms. It is strange as the same naming convention seem to work fine in other scripts.

This PR is to update the path names ending from _v* to _v

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DQMOffline/Trigger
HLTriggerOffline/Top

@kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please review it and eventually sign? Thanks.
@battibass, @mtosi, @jhgoh, @calderona, @HuguesBrun, @trocino, @rociovilar 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

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20213/213

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-20213/213/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/PR-20213/213/git-diff.patch | patch -p1

In future, you can run scram build code-checks to apply code checks

@mtosi
Copy link
Contributor

mtosi commented Aug 21, 2017

did you test your update ?
do you get filled histos ? running on which samples ?
looking at one of the most recent HLT menu [1], for instance
I see that
HLT_Mu20 is highly PS'd .. are you really interested in it ?
then, I doubt the TOP PAG is interested in path HLT_TkMu20 ... it is not available !!!

the electron paths seem to have analogous issue
where do you find HLT_Ele30_eta2p1_WPTight_Gsf ?

while, for double leptons,
are you sure TOP PAG is interested in path HLT_Mu17_TrkIsoVVL_Mu8_TrkIsoVVL ?
it seems to be highly PS'd !
and again the comment on the TkMu flavour .......

please, point me to your DQM file w/ filled histograms
then, we will start the integration test

thanks

[1]
https://cmsweb.cern.ch/confdb/#config=/cdaq/physics/Run2017/2e34/v2.2.1/HLT/V1

@mohsinwaseem
Copy link
Contributor Author

Yes I tested the update using ttbar sample for 200 events. The file is located at [1].

I will update the paths after finishing consultation with the TOP PAG. If you say, I can close this PR and open new when I am done?

[1]/afs/cern.ch/user/m/mather/public/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root

@mtosi
Copy link
Contributor

mtosi commented Aug 21, 2017 via email

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #20213 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20213/316

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #20213 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

Pull request #20213 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20213/413

@mtosi
Copy link
Contributor

mtosi commented Aug 31, 2017

@mohsinwaseem are you sure about the last 3 commits ?

@mtosi
Copy link
Contributor

mtosi commented Aug 31, 2017

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 31, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22613/console Started: 2017/08/31 15:03

@mohsinwaseem
Copy link
Contributor Author

@mtosi
I did not intend to do these commits. What I trying was to commit changed to 92X .. (backporting) but it somehow went into this PR.

Is it possible to disregard todays commits?

@mtosi
Copy link
Contributor

mtosi commented Aug 31, 2017

thanks to @fwyzard
git push -f my-cmssw 4d2122781ed63b746c1041d4ba094e7c5e640c78:from-CMSSW_9_3_0_pre3

ps:
next time, please
do
git checkout -b <NAME OF YOUR BRANCH>

@mtosi
Copy link
Contributor

mtosi commented Aug 31, 2017

ah, I forgot, the 4d2122781ed63b746c1041d4ba094e7c5e640c78 comes from the last commit you made and you want to keep

@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-20213/22613/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: 2650806
  • DQMHistoTests: Total failures: 479
  • DQMHistoTests: Total nulls: 25
  • DQMHistoTests: Total successes: 2650113
  • DQMHistoTests: Total skipped: 189
  • DQMHistoTests: Total Missing objects: 0
  • Checked 107 log files, 14 edm output root files, 26 DQM output files

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 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)

mtosi added a commit to mtosi/cmssw that referenced this pull request Sep 4, 2017
@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit c8d9643 into cms-sw:master Sep 6, 2017
mtosi added a commit to mtosi/cmssw that referenced this pull request Oct 17, 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

5 participants