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

Update Totem T2 mapping, fix bug in VFAT footprint check, and implement suggestions from PR #41777 #75

Merged
merged 2 commits into from
Jun 10, 2023

Conversation

oljemark
Copy link

@oljemark oljemark commented May 29, 2023

PR description:

For the TOTEM special foreseen on 25.6.2023, the new T2 telescope will be used (and removed after the run) to measure inelastic rate and veto diffractive p+p collisions, when measuring the elastic p+p cross section.
Previously, PR cms-sw#41472 introduced the 2-channel unpacking for T2 and PR cms-sw#41777 included some T2 DQM plots and fixes. As part of the approval for PR cms-sw#41777 , some fixes and syntax safety improvements were asked for, see cms-sw#41777 (comment) . This meant that the tracked T2 parameter useOlderT2TestFile was added to several python config files in EventFilter/CTPPSRawToDigi, and also some other cosmetic changes to those files.
Last sunday @perrotta wrote a PR (cms-sw#41784, cms-sw#41785 ) to implement the asked-for changes, from which I include most changes into this PR.

Last week we also obtained new T2 test files with a new mapping (the final version), and a more diverse set of emulated T2 data frames. (Run 368080 had LE distributed around 5 and TE ~ 12 for all events, Run 368081 same LE value but TE ~9, and Run 368082 same as 368080 but with tracks in half the planes only). Testing the code with these new files exposed several more bugs in the T2 code in the common PPS unpacker.

On receiving more feedback from the detector experts, it turned out that one more member needed to be added to the T2Digi definition in DataFormats/TotemReco , to access header flags. @vavati also suggested that the DQM additions in PR cms-sw#41777 needed to be expanded by adding the T2 as a DQMCalibrationSource, necessitating changes to RecoPPS/Configuration, also including the XML T2 geometry in a RecoPPS/Local T2 config file since the geometry was used in both the T2Rechit producer and T2 DQM module, but was not found in the database as far as we looked (thanks, @grzanka !) . The multichannel unpacker from PR cms-sw#41472 was also turned on by default, needing era modifiers for 2016-2018 PPS data taken without the T2 detector, for which the dummy mapping file mapping_totem_nt2_2021.xml caused an exception since it was copied from 2017 diamond mapping without the changes in T2 xml schema introduced in PR cms-sw#41472.

At present the T2 CRC field in the VFAT frame in the test files are still just a constant stub, but in the next version of the T2 firmware expected within a few days this will be calculated, and can validate the present calculation in the code. The code is using the same CRC formula as the RP, but due to the inverted order of reading the VFAT frame between T2 & Roman Pots, this needed new helper functions, as did the VFAT signature 'A','C','E' header word intializer checked by the footprint helper.

PR validation:

A subset of runTheMatrix tests relevant for the PPS detectors that might be affected by our code changes was run successfully after the era modifier fix noted above. Those were 136.793 & 1041 & 1042. The T2 unit test under RecoPPS/Local passed, while previously one unit test in in CalibPPS/TimingCalibration & 25 DQM/Integration were seen to fail when run on lxplus due to the files being on tape, but succeed when cmsbot/Jenkins ran on locally cached copies, see this comment: cms-sw#41785 (comment)

The PR cms-sw#41785 still passed the tests even so ( cms-sw#41785 (comment) ), although more changes have been added than is included there.

The T2 unit test in RecoPPS/Local was successfully run with the older definition & xml mapping, producing T2Digis, and likewise the totemt2_dqm_test_nino_cfg.py script on last week's new files, produced the expected DQM plots. The totemt2_dqm_test_common_cfg.py script was run on PPS alignment run data with the other PPS detectors connected but not T2, showing that even though we include T2 in the common CTPPS RawToDigi and Reconstruction tasks, this does not negatively affect the other PPS detectors.

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:

These three PR's, cms-sw#41472 , cms-sw#41777 and this one should be backported either to a special release on the 13_0_X cycle or the appropriate regular release on the 13_0_X . At a lower priority, backporting will also be done to 13_1_X.

@oljemark
Copy link
Author

oljemark commented May 31, 2023

Hi, @grzanka . So if we want the T2 added to the calibration source that needs to be uncommented to be used, should we still add the T2 to some ctpps-task around here: https://github.com/CTPPS/cmssw/blob/fo_nt2_mapfix2/RecoPPS/Configuration/python/recoCTPPS_cff.py#L16 ? And what about keeping the produced data classes, in https://github.com/CTPPS/cmssw/blob/fo_nt2_mapfix2/RecoPPS/Configuration/python/RecoCTPPS_EventContent_cff.py ?

@grzanka
Copy link

grzanka commented May 31, 2023

Hi, @grzanka . So if we want the T2 added to the calibration source that needs to be uncommented to be used, should we still add the T2 to some ctpps-task around here: https://github.com/CTPPS/cmssw/blob/fo_nt2_mapfix2/RecoPPS/Configuration/python/recoCTPPS_cff.py#L16 ? And what about keeping the produced data classes, in https://github.com/CTPPS/cmssw/blob/fo_nt2_mapfix2/RecoPPS/Configuration/python/RecoCTPPS_EventContent_cff.py ?

You need a DQM source https://github.com/CTPPS/cmssw/blob/fo_nt2_mapfix2/DQM/CTPPS/python/totemT2DQMSource_cfi.py
which expects totemT2Digis and totemT2RecHits

Somewhere in the execution chain you need to add modules which produce these objects.

I believe adding necessary digitizer and rechit producer is needed in couple of places in the RecoPPS/Local, for example in python directory.

@oljemark oljemark force-pushed the fo_nt2_mapfix2 branch 3 times, most recently from e177927 to abecc3e Compare June 7, 2023 01:43
@oljemark oljemark merged commit 609dc7c into master Jun 10, 2023
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.

2 participants