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

DQM new pixel residuals #13240

Merged
merged 9 commits into from Feb 16, 2016

Conversation

schneiml
Copy link
Contributor

The current pixel DQM does not collect residuals data for the pixel endcaps (though the plots are booked).

This change creates new pixel residual plots based on the SiStrip residual code (TrackerMonitorTrack, which in turn is based on Alignment/OfflineValidation). The code was also extended to create plots for the Y direction, which are not very useful for the strips but now also generated. For the pixels, plots are generated for each layer and each disk, note that the disk plots appear in the first half cylinder folder even though they cover the full disk.

The old pixel residual code was left in place.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @schneiml (Marcel Schneider) for CMSSW_8_0_X.

It involves the following packages:

DQM/TrackerMonitorTrack

@cmsbuild, @vanbesien, @deguio, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @threus this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@boudoul
Copy link
Contributor

boudoul commented Feb 10, 2016

thanks @schneiml ! we are encouraging you to add README.md files per package in order to describe the packages functionality (in particular what the codes you wrote is actually doing) , could you please add it ?
Thanks a lot
@ fioriNTU : FYI

@cmsbuild
Copy link
Contributor

Pull request #13240 was updated. @cmsbuild, @vanbesien, @deguio, @davidlange6 can you please check and sign again.

@deguio
Copy link
Contributor

deguio commented Feb 12, 2016

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11166/console

@cmsbuild
Copy link
Contributor

-1
Tested at: b974bdc
When I ran the RelVals I found an error in the following worklfows:
140.53 step1

DAS Error

1000.0 step1

DAS Error

1001.0 step1

DAS Error

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13240/11166/summary.html

@boudoul
Copy link
Contributor

boudoul commented Feb 15, 2016

hi @deguio , it seems that the tests are failing due to DAS error, could you please retrigger the tests ? It would be great to get those DQM plots for the next data taking...
Thanks

@davidlange6
Copy link
Contributor

please test

On Feb 15, 2016, at 5:07 AM, boudoul notifications@github.com wrote:

hi @deguio , it seems that the tests are failing due to DAS error, could you please retrigger the tests ? It would be great to get those DQM plots for the next data taking...
Thanks


Reply to this email directly or view it on GitHub.

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11203/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@boudoul
Copy link
Contributor

boudoul commented Feb 16, 2016

hi @deguio , do you think this can be signed by today ? Sorry to ask, but would be great to have in the release for data taking (MWGR) - thanks and ciao.

@deguio
Copy link
Contributor

deguio commented Feb 16, 2016

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@deguio
Copy link
Contributor

deguio commented Feb 16, 2016

this can go in, but for the next iteration I have two comments:

  • I still see that the DQMStore is instantiated directly. this should go away and it doesn't affect the code functionality
  • is the old residual code still needed? my understanding is that it can be removed.

thanks,
F.

@schneiml
Copy link
Contributor Author

  • Regarding the DQMStore and other "style" issues: There is more to do, I tried to keed the diff minimal.
  • Regarding the old code: the generated plots are not at all identical, since the definitions of "residual" are quite different. Thats why I think the old ones should stay there until we know that the new code does the right thing.

In the TK DQM meeting today we discussed some changes. I have two more commits affecting the folder layout. Should these go in this PR as well?

I am currently investigating with @fioriNTU what the differences between the old and new "residuals" are and which definition we actually want, but this will need some more time.

@davidlange6
Copy link
Contributor

+1

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