-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add SiStripCalCosmics ALCANANO #34985
Add SiStripCalCosmics ALCANANO #34985
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34985/24829
|
A new Pull Request was created by @pieterdavid (Pieter David) for master. It involves the following packages:
@perrotta, @malbouis, @yuanchao, @jordan-martins, @davidlange6, @chayanit, @bbilin, @wajidalikhan, @kpedro88, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @tlampen, @qliphy, @pohsun, @francescobrivio, @fabiocos, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters:
|
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e48dd/17976/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34985/24864
|
Pull request #34985 was updated. @perrotta, @malbouis, @yuanchao, @jordan-martins, @davidlange6, @chayanit, @bbilin, @wajidalikhan, @kpedro88, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @tlampen, @qliphy, @pohsun, @francescobrivio, @fabiocos, @tvami can you please check and sign again. |
Hi @cms-sw/pdmv-l2 colleagues, what's your take of this PR? It's been around for a while now :) |
+1 |
+Upgrade |
@mariadalfonso would you mind signing this again? The PR has been rebased since you did it already |
+xpog |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged. |
I agree that it would be nice to have this for CRAFT, i.e. +1 for the backport |
PR description:
This PR adds NanoAOD-style output for the SiStripCalCosmics ALCARECO (introduced in #26032), based on the existing configuration (which was run by the DPG) to make flat trees for the Lorentz angle and backplane correction measurement, and taking advantage of the changes in #33565 .
The code adds a plugin for per-run per-module information about the magnetic field etc., uses a
SimpleFlatTableProducer
for per-track information, and adds a plugin that stores the necessary per-cluster information (split in a base class that gets the clusters, and a subclass that stores the variables for the Lorentz angle and backplane correction - another subclass will be used for the gains tree).This is a very small ALCARECO, so the size is small in both cases (the flat format is about the same size as the EDM format), but it has been extensively validated, and provides an interesting case to discuss the configuration and possibility of central production.
As a minimal solution to add this to cmsDriver I added a
SiStripCalCosmicsNano
ALCARECO based on the existingSiStripCalCosmics
, and changed the code to insert aNanoAODOutputModule
if the dataTier of a FilteredStream is NANOAOD. That seems to work (it can be tested with theALCA:SiStripCalCosmicsNano
step, which is also added to the relevant matrix workflows by this PR), but something less ad-hoc may be preferred - please provide input, this is the reason why the PR is marked as "work in progress".The idea (discussed with XPOG) is to have this integrated for testing at T0 with CRAFT in October, so depending on if that works out and which release is used there, a backport to the 12_0 branch may be needed.
PR validation:
Event-by-event comparison for 2018 cosmics by @robervalwalsh , see these slides
CC: @robervalwalsh @mdelcourt @ataliercio @jandrejk @mmusich @tsusa @mariadalfonso @gouskos