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/BeamMonitor] fix y-axis range to properly display d0-phi plot #10769

Merged

Conversation

sarafiorendi
Copy link
Contributor

We found y-axis range to be too narrow, causing the sinusoidal d0-phi distribution to be capped.
The attached plots show the effect of a too narrow y-axis range.

hd0phi0
hd0phi0_narrowrange

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @sarafiorendi for CMSSW_7_6_X.

[DQM/BeamMonitor] fix y-axis range to properly display d0-phi plot

It involves the following packages:

DQM/BeamMonitor

@cmsbuild, @danduggan, @deguio can you please review it and eventually sign? Thanks.
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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@rmanzoni
Copy link
Contributor

adding @diguida and @mmusich

@mmusich
Copy link
Contributor

mmusich commented Aug 14, 2015

please test
don't know if I can summon the bot outside my small backyard

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@davidlange6
Copy link
Contributor

@sarafiorendi - it would be worth rewriting this to be independent of knowing the range of values expected. Something like https://en.wikipedia.org/wiki/Algorithms_for_calculating_variance would take less memory (two 1D histograms with N bins instead of a profile with Nx100 bins) and be robust against both the predefined limits and the effect of the finite bin size in the TProfile.

Eg - follow this sort of algorithm (which needs a 1D histogram for "Sum" and a histogram for "Sum_sqr")
and then to be generalized for bins of phi.

def shifted_data_variance(data):
if len(data) == 0:
return 0
K = data[0]
n = 0
Sum = 0
Sum_sqr = 0
for x in data:
n = n + 1
Sum += x - K
Sum_sqr += (x - K) * (x - K)
variance = (Sum_sqr - (Sum * Sum)/n)/(n - 1)

use n instead of (n-1) if want to compute the exact variance of the given data

use (n-1) if data are samples of a larger population

return variance

@diguida
Copy link
Contributor

diguida commented Aug 14, 2015

@davidlange6
thanks for your comment. Actually, we think that such a range is a "maximum" limit. We can assume that a d0 movement outside [-0.5,0.5] cm would be a clear sign that something is going wrong either in tracking or, most likely, in the LHC (they have instabilities on the IP setting).
We can let this enter as a hotfix.

@davidlange6
Copy link
Contributor

On Aug 14, 2015, at 12:45 PM, Salvatore Di Guida notifications@github.com wrote:

@davidlange6
thanks for your comment. Actually, we think that such a range is a "maximum" limit. We can assume that a d0 movement outside [-0.5,0.5] cm would be a clear sign that something is going wrong either in tracking or, most likely, in the LHC (they have instabilities on the IP setting).
We can let this enter as a hotfix.

ah - sure -I commented as in the fix we have given up a big factor in resolution on the measurement. hopefully that does not matter too much..


Reply to this email directly or view it on GitHub.

@rmanzoni
Copy link
Contributor

@davidlange6 ah, you're right, I got your point.
Given the hot-fix nature of this change, maybe we can just (reasonably) increase the number of bins for now.

@mmusich
Copy link
Contributor

mmusich commented Aug 14, 2015

@davidlange6 @rmanzoni, but are we actually measuring anything out of this histogram? Sorry but I don't understand. The [-0.5,0.5] is taken directly from another similar piece of DQM code: https://github.com/cms-sw/cmssw/blob/CMSSW_7_6_X/DQM/TrackingMonitor/python/TrackingMonitor_cfi.py#L322 if the number of bins is an argument, it should be true also there

@rmanzoni
Copy link
Contributor

@mmusich no, it's not meant for anything else than monitoring purposes and we don't measure anything out of it. All considered, the range and number of bins should be sensible as they are already.

@cmsbuild
Copy link
Contributor

@deguio
Copy link
Contributor

deguio commented Aug 14, 2015

+1

@cmsbuild
Copy link
Contributor

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

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Aug 15, 2015
…Range

[DQM/BeamMonitor] fix y-axis range to properly display d0-phi plot
@cmsbuild cmsbuild merged commit 73a51d6 into cms-sw:CMSSW_7_6_X Aug 15, 2015
@rmanzoni rmanzoni deleted the CMSSW_7_6_X_fixD0PhiRange branch August 19, 2015 14:22
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

7 participants