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

Int monitor element fix #4868

Merged
merged 9 commits into from Aug 14, 2014
Merged

Conversation

aehart
Copy link
Contributor

@aehart aehart commented Aug 2, 2014

Converted int monitor elements to TH1 to fix a problem with harvesting int monitor elements.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2014

A new Pull Request was created by @aehart (Andrew Hart) for CMSSW_7_2_X.

Int monitor element fix

It involves the following packages:

DQM/SiPixelMonitorClient
DQM/SiPixelMonitorRawData

@ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please review it and eventually sign? Thanks.
@threus this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2014

@deguio
Copy link
Contributor

deguio commented Aug 13, 2014

@aehart
which kind of problem with ints in harvesting are you referring to?
thanks,
F.

@aehart
Copy link
Contributor Author

aehart commented Aug 13, 2014

I understand that during harvesting, the int monitor elements were not
being added as they should have been, but instead only the value from
the final file was being used. Switching to TH1 monitor elements fixes
the problem. I've been working with Duncan on this issue, so perhaps he
can elaborate further.

Cheers,
Andrew

On 08/13/2014 11:20 AM, deguio wrote:

@aehart https://github.com/aehart
which kind of problem with ints in harvesting are you referring to?
thanks,
F.


Reply to this email directly or view it on GitHub
#4868 (comment).

@leggat
Copy link
Contributor

leggat commented Aug 13, 2014

Hi @deguio

It was Dan that told me about the harvesting problem here - the Int type monitor elements were, as Andrew says, not actually being summed at all in the step3 file combination. As we were storing various related error parameters in arrays of int MEs it made sense to convert them to TH1s. I also think it's a better way to store the information aesthetically, but that's not the reason for the update...

Cheers,
Duncan

@danduggan
Copy link
Contributor

@leggat This is great and a big help to pixel FED error monitoring. Thanks to you and @aehart for making this change.

@deguio
Copy link
Contributor

deguio commented Aug 14, 2014

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine).

ktf added a commit that referenced this pull request Aug 14, 2014
@ktf ktf merged commit 79a8b54 into cms-sw:CMSSW_7_2_X Aug 14, 2014
@aehart aehart deleted the int_monitor_element_fix branch November 19, 2014 06:43
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