Update of the Zmumu validation tool #25159

cardinia · 2018-11-08T14:26:17Z

@connorpa
Implemented features:
2D plots Mass vs EtaPhi of muon (plus or minus)
Plotting options regarding the Y axis range in 1D histograms

Features presented in:
https://indico.cern.ch/event/768246/#72-latest-developments-of-vali
https://indico.cern.ch/event/688851/#sc-28-2-improvement-of-z-valid

…ng the Y axis range and 2D plots

cmsbuild · 2018-11-08T14:26:37Z

A new Pull Request was created by @cardinia for CMSSW_10_2_X.

It involves the following packages:

Alignment/OfflineValidation
MuonAnalysis/MomentumScaleCalibration

@tocheng, @monttj, @cmsbuild, @franzoni, @fgolf, @pohsun, @peruzzim, @lpernie can you please review it and eventually sign? Thanks.
@adewit, @tocheng, @argiro, @bellan, @tlampen, @mschrode, @mmusich this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

Backported from Update of the Zmumu validation tool, pull request to master branch #25220

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Base.C

vargasa · 2018-11-08T17:23:25Z

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Base.C

@@ -220,15 +235,15 @@ void splitOptionRecursive(string rawoption, vector<string>& splitoptions, char d
 }

 void getCustomRanges(TString type, TString resonance, int iP, float minmax_plot[2]);
-void MultiHistoOverlapAll_Base_one(string files, string labels, string colors, string linestyles, string markerstyles, TString directory, TString resonance, TString type, bool switchONfit);
+void MultiHistoOverlapAll_Base_one(string files, string labels, string colors, string linestyles, string markerstyles, TString directory, TString resonance, TString type, bool switchONfit, bool AutoSetRange, float CustomMinY, float CustomMaxY);


I'd suggest to set default values to the arguments added making sure they work as expected when they were not there. In that way, previously written code that uses this class will still work :)

Dear Andres,

I assigned a default value in the function definition, so I cannot assign it also in the function declaration. I could assign the default value in the function declaration and remove it from the function definition though, is that what you were suggesting?

Thanks in advance,
Andrea

I could assign the default value in the function declaration and remove it from the function definition though, is that what you were suggesting?

Not exactly. But that's a good idea.

Also, please make sure your commit description offers an explanation of the change you're applying and make sure it is unique. Right now I see you have three different commits with the same description. It is better to avoid 'Merge' commits, check git rebase

Thank you very much for the tip, I'll implement the change.
As for the commits I'll try to be more careful next time when preparing a pull request, thanks for the information.

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Y1S.C

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Z.C

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Base.C

cmsbuild · 2018-11-09T10:24:37Z

Pull request #25159 was updated. @tocheng, @monttj, @cmsbuild, @franzoni, @fgolf, @pohsun, @peruzzim, @lpernie can you please check and sign again.

mmusich · 2018-11-09T10:32:42Z

MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/MultiHistoOverlapAll_Base.C

+	      if(value>zMax)zMax=value;
+	    }
+	  }
+	  maxDist=fabs(zMax-Mass);


@cardinia you are using here a variable zMax which is defined out of this scope (in the if(type=="mean"){ clause)
How is this supposed to work?

Dear Marco,

I am sorry but I do not understand what you mean: the zMax and zMin variables are defined within the scope of the condition regarding the type of the histogram, and the scope of that condition ends at line 415.

I defined the variables in that scope since they are necessary only to fix the z axis range when working with the average of the mass distribution and not the standard deviation.

Can you be more specific regarding the issue you noticed?

Thanks in advance,
Andrea

Hi Andrea, I counted the parentheses wrong -my bad.
Still I am wondering about the blank stripe you showed in the middle of the sigma distribution during the meeting (see also comment at line https://github.com/cms-sw/cmssw/pull/25159/files/88635fbdd021ffe44198e59bc13b66a59ecca03f#diff-b7a5bcf180f203de7abd9bb4d084e665R370).
In the case of the sigma which rangeuser is applied? None?

Hi Marco,
no range is set for the sigma distribution, the plots are identical to what you would observe in the BiasCheck.root file drawing the histograms with the colz option.
I'm investigating the code in CompareBiasZValidation.cc [1] to try and understand what causes the problem, as of now I did not find anything weird...
Anyway I put the comment you underlined in the code as a reminder that there is still work to be done, then I decided to make the pull request anyway given that some features can already be used in the framework.
Cheers,
Andrea

[1]https://github.com/cms-sw/cmssw/blob/02d4198c0b6615287fd88e9a8ff650aea994412e/MuonAnalysis/MomentumScaleCalibration/test/Macros/RooFit/CompareBiasZValidation.cc

…m/cardinia/cmssw into from-CMSSW_10_2_X_2018-11-08-1100

cmsbuild · 2018-11-09T11:05:10Z

Pull request #25159 was updated. @tocheng, @monttj, @cmsbuild, @franzoni, @fgolf, @pohsun, @peruzzim, @lpernie can you please check and sign again.

fabiocos · 2018-11-12T09:06:48Z

@cardinia sorry, but is this the backport of which PR? Please provide a master version of this development

cardinia · 2018-11-13T15:34:45Z

@fabiocos sorry, my bad. I accidentally made the PR to this branch instead of the master. I'm now trying to make the rebase to then create a proper PR to the master version, however I'm having some difficulties in doing so. Should I close this PR and open a new one or is it possible to change the branch destination of this commit?

cardinia · 2018-11-13T16:55:29Z

The pull request to the master branch has now been made (#25220). This pull request can either be used for backporting or closed entirely.
Sorry for the confusion.

fabiocos · 2018-11-15T12:01:51Z

@cardinia well it is up to you and Alignment people to say whether this is needed in 10_3_X for HI activities or not...

mmusich · 2018-11-15T13:23:15Z

Am I wrong or is this backport to 10.2 rather than 10.3? I don't think this is useful in 10.3, while it might in 10.2 where the analysis of 2018 pp data is still ongoing, in case a new release is built

fabiocos · 2018-11-24T17:36:41Z

please test

cmsbuild · 2018-11-24T17:36:56Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31836/console Started: 2018/11/24 19:03

cmsbuild · 2018-11-24T20:20:35Z

+1
Tested at: 25fed82
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25159/31836/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
25fed82
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25159/31836/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25159/31836/git-merge-result

cmsbuild · 2018-11-24T20:20:37Z

Comparison job queued.

cmsbuild · 2018-11-24T22:19:05Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25159/31836/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2986766
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2986574
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 129 log files, 14 edm output root files, 31 DQM output files

pohsun · 2018-11-25T18:30:54Z

+1

fabiocos · 2018-11-26T09:27:27Z

+1

fabiocos · 2018-11-26T09:27:52Z

merge

utilities for alignment studies

fabiocos · 2018-11-26T09:29:45Z

backport #25220

cardinia added 3 commits November 8, 2018 14:55

Implemented Zmumu validation plotting options for automatically setti…

b0a6d42

…ng the Y axis range and 2D plots

Implemented Zmumu validation plotting options for automatically setti…

dc4f6e0

…ng the Y axis range and 2D plots

Implemented Zmumu validation plotting options for automatically setti…

1552cb5

…ng the Y axis range and 2D plots

cmsbuild added this to the CMSSW_10_2_X milestone Nov 8, 2018

cmsbuild added alca-pending analysis-pending comparison-pending orp-pending pending-signatures tests-pending labels Nov 8, 2018

vargasa reviewed Nov 8, 2018

View reviewed changes

Update MultiHistoOverlapAll_Base.C

88635fb

mmusich reviewed Nov 9, 2018

View reviewed changes

cardinia added 3 commits November 9, 2018 11:53

Fixed bugs in function declaration

e8d9756

Fixed bugs in function declaration

6402f1c

Merge branch 'from-CMSSW_10_2_X_2018-11-08-1100' of https://github.co…

25fed82

…m/cardinia/cmssw into from-CMSSW_10_2_X_2018-11-08-1100

cardinia mentioned this pull request Nov 13, 2018

Update of the Zmumu validation tool, pull request to master branch #25220

Merged

cmsbuild added tests-started and removed tests-pending labels Nov 24, 2018

cmsbuild added tests-approved and removed tests-started labels Nov 24, 2018

cmsbuild added comparison-available and removed comparison-pending labels Nov 24, 2018

cmsbuild added alca-approved and removed alca-pending labels Nov 25, 2018

cmsbuild added orp-approved and removed orp-pending labels Nov 26, 2018

cmsbuild merged commit 2ce2b3b into cms-sw:CMSSW_10_2_X Nov 26, 2018

cmsbuild added the backport-ok label Nov 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update of the Zmumu validation tool #25159

Update of the Zmumu validation tool #25159

cardinia commented Nov 8, 2018

cmsbuild commented Nov 8, 2018 •

edited

Loading

vargasa Nov 8, 2018

cardinia Nov 9, 2018

vargasa Nov 9, 2018 •

edited

Loading

cardinia Nov 12, 2018

cmsbuild commented Nov 9, 2018

mmusich Nov 9, 2018

cardinia Nov 9, 2018

mmusich Nov 9, 2018

cardinia Nov 9, 2018 •

edited

Loading

cmsbuild commented Nov 9, 2018

fabiocos commented Nov 12, 2018

cardinia commented Nov 13, 2018

cardinia commented Nov 13, 2018

fabiocos commented Nov 15, 2018

mmusich commented Nov 15, 2018

fabiocos commented Nov 24, 2018

cmsbuild commented Nov 24, 2018 •

edited

Loading

cmsbuild commented Nov 24, 2018

cmsbuild commented Nov 24, 2018

cmsbuild commented Nov 24, 2018

pohsun commented Nov 25, 2018

fabiocos commented Nov 26, 2018

fabiocos commented Nov 26, 2018

fabiocos commented Nov 26, 2018

Update of the Zmumu validation tool #25159

Update of the Zmumu validation tool #25159

Conversation

cardinia commented Nov 8, 2018

cmsbuild commented Nov 8, 2018 • edited Loading

vargasa Nov 8, 2018

Choose a reason for hiding this comment

cardinia Nov 9, 2018

Choose a reason for hiding this comment

vargasa Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

cardinia Nov 12, 2018

Choose a reason for hiding this comment

cmsbuild commented Nov 9, 2018

mmusich Nov 9, 2018

Choose a reason for hiding this comment

cardinia Nov 9, 2018

Choose a reason for hiding this comment

mmusich Nov 9, 2018

Choose a reason for hiding this comment

cardinia Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

cmsbuild commented Nov 9, 2018

fabiocos commented Nov 12, 2018

cardinia commented Nov 13, 2018

cardinia commented Nov 13, 2018

fabiocos commented Nov 15, 2018

mmusich commented Nov 15, 2018

fabiocos commented Nov 24, 2018

cmsbuild commented Nov 24, 2018 • edited Loading

cmsbuild commented Nov 24, 2018

cmsbuild commented Nov 24, 2018

cmsbuild commented Nov 24, 2018

pohsun commented Nov 25, 2018

fabiocos commented Nov 26, 2018

fabiocos commented Nov 26, 2018

fabiocos commented Nov 26, 2018

cmsbuild commented Nov 8, 2018 •

edited

Loading

vargasa Nov 9, 2018 •

edited

Loading

cardinia Nov 9, 2018 •

edited

Loading

cmsbuild commented Nov 24, 2018 •

edited

Loading