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

Merge SiStrip Hit resolution code in cmssw #40754

Merged
merged 9 commits into from
Feb 27, 2023

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Feb 13, 2023

PR description:

In the view of providing a dedicated SiStrip Hit resolution workflow to be run at the Prompt Calibration Loop, I start to port into cmssw the bulk of the standalone Strip hit resolution code from a private repository (https://gitlab.cern.ch/coldham/hitresolutionproject/).
The PR is RFC for the time being to start integrating comments and identify possible code issues, while integrating more code.

PR validation:

I've added some basic unit test functionality to demonstrate the code runs correctly: scram b runtests use-ibeos

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Probably will be backported together with other developments in 13_0_X to be use for 2023 data-taking analysis.

cc: @mdelcourt

KathrynColdham and others added 3 commits January 30, 2020 15:33
move everything up

Remove a few (output) files, fix config

Resolution values are printed and also written to an output text file.

Binning for the residuals changed from 20 to 40 and gaus fit applied

Need to identify where the difference between the two positions of hit measurements is calculated.

Resolution values are nans - need to resolve.

Changed the names of two of the plots

Variables used to fill the histograms have been changed.

Changed how the resolution was calculated.

Corrected a minor mistake

Numerator changed to just sigma^2(delta_pred).

Improved resolution values obtained.

Unit conversion to cm and strip units applied

Make both cm and strip unit plots, add residuals

Compile fixes for newer CMSSW versions

Whitespace cleanup and code modernisation: auto, range-based for loops, and no C-style casts

Add a config for running HitResol on UL SiStripCalCosmics ALCARECO

A ROOT macro for calculating the hit resolution values by using hitresol.root as input.

An output root file to see the gaussian fitted distributions.

Outputs obtained for each tracker region. Values all nans - need to investigate.

deleted: ResolutionValues_315252.txt

Outputting more info into the text file.

More columns added to the output text file

Started to change the histogram ranges.

Mistake corrected in RDataFrames.

Added more comments

HitDX seems to be too large.

Script outputs the resolution values for both units, but need to investigate why HitDX is so large

More criteria included.

Modifying the config file to output files with different names for UL or ALCARECO.

Mistake corrected

Started to write the README

Mistake corrected

Title corrected

Started to add subheadings

Tried to include the pair method equation

Started to include the parameters. Need to include more and sort out the equations

Started to include permalinks in the README

More permalinks included

Mistake corrected

Permalink for the at edge parameter included

Small mistake corrected

More info included in the section about how to run the code

Mistake corrected with the bullet points

Started to include references but need to add permalinks

Resources added.

Subdirectory created

Started to modify the script for different input file names.

Permalinks added for the references.

Minor wording mistake corrected

Added author names next to the files

Trying to insert the equations correctly

Correcting the format of the equations

All equations corrected

Corrected the equations in the parameters section

Wording tweaked

Commit of the results but still get nan resolution values.

Mistakes corrected for writing to output files

Updated to include info on how to change the units and sample type.

Added the criteria of clusters must not have the same width and that the clusters must have a width of < 4 strips

Permalinks to two of the criteria added

Small mistake corrected

Another small mistake corrected

Typo corrected

Another criterion included

Included another permalink

Started to include criteria from Keith Ulmer's presentation.

Small typo corrected

Separate momentum criteria for strips and pixels included.

Permalinks updated

One of the criteria removed

Still get nan values for the resolutions

Output text files made a little clearer

Results for pixels included.

Two of the permalinks modified

Pixels included

Trying to resolve issue with side, ring and wheel regions

Included the output for all TOB, all TIB, all TID and all TEC

Typo corrected

Mistake corected but stil get zeroes for the TID and TEC regions

Values updated but still the same issues

Cut flow reports added

Cut flow reports moved to a sub directory

Subdirectories created for the outputs

Added more info

Resolution values improved but are 0 for some regions

Partition definitions changed

Definition for TEC all changed, not sure if correct

Partitions modified

Commit of a plot of the hit resolution values

Deleted png file

Updated the README to reflect the changes made to the pair method equation

Mistake corrected

Permalinks updated

Unit label changed to centimetres.

Updated the description for the output files

Added permalinks for the description of the output directories.

Included permalinks for the partition definitions

Made the README a bit clearer

Another improvement made to the README

Added a description about the ResolutionPlots plotting script

More instructions added

Strip units changed to pitch units

Small mistake corrected

Another change made for units

Code modified to produce hit resolution values in pitch units

Resolution values plotted for pitch units

Cluster width included in the output root file

Binning improved

clean-up
@mmusich
Copy link
Contributor Author

mmusich commented Feb 13, 2023

assign alca

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40754/34171

  • This PR adds an extra 80KB to repository

@cmsbuild
Copy link
Contributor

New categories assigned: alca

@yuanchao,@francescobrivio,@malbouis,@saumyaphor4252,@tvami,@ChrisMisan you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

  • CalibTracker/SiStripHitResolution (****)

The following packages do not have a category, yet:

CalibTracker/SiStripHitResolution
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks.
@echabert, @robervalwalsh, @gbenelli this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor Author

mmusich commented Feb 14, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: HeaderConsistency RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ba1dbb/30619/summary.html
COMMIT: b582811
CMSSW: CMSSW_13_1_X_2023-02-13-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40754/30619/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • You potentially removed 28 lines from the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3555972
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555950
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ba1dbb/30911/summary.html
COMMIT: 85356ec
CMSSW: CMSSW_13_1_X_2023-02-26-0000/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40754/30911/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 20834.020834.0_TTbar_14TeV+2026D88/step2_TTbar_14TeV+2026D88.log
  • 20834.2120834.21_TTbar_14TeV+2026D88_ProdLike/step2_TTbar_14TeV+2026D88_ProdLike.log
  • 20834.10320834.103_TTbar_14TeV+2026D88Aging3000/step2_TTbar_14TeV+2026D88Aging3000.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • You potentially added 2 lines to the logs
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3528955
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3528922
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor Author

mmusich commented Feb 26, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ba1dbb/30912/summary.html
COMMIT: 85356ec
CMSSW: CMSSW_13_1_X_2023-02-26-0000/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40754/30912/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 20834.020834.0_TTbar_14TeV+2026D88/step2_TTbar_14TeV+2026D88.log
  • 20834.10320834.103_TTbar_14TeV+2026D88Aging3000/step2_TTbar_14TeV+2026D88Aging3000.log
  • 20834.2120834.21_TTbar_14TeV+2026D88_ProdLike/step2_TTbar_14TeV+2026D88_ProdLike.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3528955
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3528926
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Feb 26, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ba1dbb/30914/summary.html
COMMIT: 85356ec
CMSSW: CMSSW_13_1_X_2023-02-26-0000/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/40754/30914/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 20834.020834.0_TTbar_14TeV+2026D88/step2_TTbar_14TeV+2026D88.log
  • 20834.10320834.103_TTbar_14TeV+2026D88Aging3000/step2_TTbar_14TeV+2026D88Aging3000.log
  • 20834.2120834.21_TTbar_14TeV+2026D88_ProdLike/step2_TTbar_14TeV+2026D88_ProdLike.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3528955
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3528926
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor Author

mmusich commented Feb 27, 2023

After three relval input failures due to DAS, I guess there is no point in trying further.

@mmusich
Copy link
Contributor Author

mmusich commented Feb 27, 2023

@cms-sw/alca-l2 can you please re-sign here (first signature buried by gitHub, available here?
The last commit 85356ec just cleans up include files, as requested at #40754 (review)

@francescobrivio
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

merge

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.

7 participants