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

Add FEDRawDataCollection to RAWMINIAOD skim event content #19846

Merged

Conversation

kfjack
Copy link
Contributor

@kfjack kfjack commented Jul 20, 2017

Adding FEDRawDataCollection to RAW+MINIAOD event content, which is used in the MuTau skim.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kfjack (Kai-Feng Chen) for master.

It involves the following packages:

Configuration/EventContent

@cmsbuild, @franzoni, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @Martin-Grunewald this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

…EventContent to avoid the hidden drop * in MicroEventContent.
@fabozzi
Copy link
Contributor

fabozzi commented Jul 22, 2017

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 22, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21687/console Started: 2017/07/22 16:10

@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-19846/21687/summary.html

Comparison Summary:

  • You potentially added 5 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 2355459
  • DQMHistoTests: Total failures: 15058
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2340235
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 93 log files, 14 edm output root files, 23 DQM output files

@davidlange6
Copy link
Contributor

hi @kfjack - seems a clear fix - but any idea how this was missed in the validation last fall (when the event content was updated previously)?

@kfjack
Copy link
Contributor Author

kfjack commented Jul 26, 2017

Hi @davidlange6 - not sure. It might be correlated with miniAOD updates. I was looking into the blame history but no clear clue. Sounds like it is better to have a tighter cross check from my side.

Do you know how to implement an automatic check for every build? Now the cross check with relval is done by myself from time to time with a private script, and it is not very smart/efficient.

@davidlange6
Copy link
Contributor

davidlange6 commented Jul 26, 2017 via email

@kfjack
Copy link
Contributor Author

kfjack commented Jul 26, 2017

Ok. So the bug was there at beginning. Apparently this is not good...
I want to do

  1. make sure all of the active skims have their output in relval.
  2. make sure the event content is correct (or the same as before).

The current lazy script in my hand is just listing relval files and check if the corresponding skim files are there, without checking the contents.

@davidlange6
Copy link
Contributor

davidlange6 commented Jul 26, 2017 via email

@kfjack
Copy link
Contributor Author

kfjack commented Jul 26, 2017

Indeed implementing those checks cannot resolve this current issue (or prevent from it for future updates; the content should be checked by skim users in any case), but better than nothing I think...

Anyway for the check one - do you have a recommended starting point? Given I still have ~few working days I can dig a little bit here.
for the check two - no clear idea for now (was thinking to compare the list of event contents with a reference version or so).

Update: just discussed with Francesco on this. We have confirmed that we did not skip any validation checks nor relval tests before changing the content, and got the GL from skim users. But somehow the this was still happening unfortunately.

@davidlange6
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit b7868eb into cms-sw:master Aug 2, 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.

4 participants