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

CTPPS: TotemTimingDigi, totemTimingRawToDigi and mapping for the new Timing detector in Vertical RPs #22796

Merged
merged 29 commits into from Apr 11, 2018

Conversation

nminafra
Copy link
Contributor

This PR introduces support for the new timing detectors (TOTEM) in the vertical roman pots (RPs).
This code will be needed for the 90m dedicated run in t0 and P5 for DQM (10_2); however, it would be very much appreciated, but (considering the time scale) only if possible, to approve it before the alignment run (17th April) so that it can be used for the DQM.

NOTE: the digital part of this detector is completely different from the CTPPS diamond detector. Here we use a sampler (SAMPIC) instead of a TDC. Therefore, even if we can share part of the unpacker (for the OptoRx frames), the dataformat is substantially different.

This PR introduces many modifications because an effective test is only possible using the TotemTimingDIGIs.

Suggested test:
EventFilter/CTPPSRawToDigi/test/test_standard_sequence_cfg.py

Data with this detector are available in:
/store/group/dpg_ctpps/comm_ctpps/TotemTiming/Minidaq/519/

Future plan:

  • DQM using DIGI in preparation, PR soon
  • Geometry of the new detectors
  • Migration to db for mapping and geometry
  • RecHitProducer: we plan to produce the CTPPSDiamondRecHits, so that the rest of the code can be common with the CTPPS Diamond detector.
  • DQM using recHits

We are reasonably sure that all the bugs were fixed, however being a completely new detector, we foresee the possibility of other "bugfixing" PR.

Thanks!

@fabferro

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @nminafra (Nicola Minafra) for master.

It involves the following packages:

CondFormats/CTPPSReadoutObjects
DQM/CTPPS
DataFormats/CTPPSDetId
DataFormats/CTPPSDigi
EventFilter/CTPPSRawToDigi

@perrotta, @ghellwig, @civanch, @vazzolini, @kmaeshima, @arunhep, @mdhildreth, @dmitrijus, @cmsbuild, @franzoni, @jfernan2, @cerminar, @slava77, @ggovi, @vanbesien, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @forthommel, @rovere, @Martin-Grunewald, @tocheng, @mmusich, @seemasharmafnal 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

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 29, 2018

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

@cmsbuild cmsbuild removed the hold label Apr 8, 2018
@perrotta
Copy link
Contributor

perrotta commented Apr 9, 2018

+1

@nminafra
Copy link
Contributor Author

@perrotta @fabiocos Is this PR mature enough to start backporting to 10_1_X? Thanks

@perrotta
Copy link
Contributor

@cerminar,@arunhep,@franzoni,@lpernie,@ghellwig, @ggovi could you please evaluate and possibly sign (if you think so) this PR?

@ggovi
Copy link
Contributor

ggovi commented Apr 10, 2018

+1

1 similar comment
@arunhep
Copy link
Contributor

arunhep commented Apr 10, 2018

+1

@cmsbuild
Copy link
Contributor

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 now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

@nminafra well, before we should deploy it in the development release and test it. Moreover this PR updates the event content in RECO format, adding the totemTimingRawToDigi:TotemTiming (TotemTimingDigiedmDetSetVector) collection

@perrotta
Copy link
Contributor

perrotta commented Apr 11, 2018 via email

@nminafra
Copy link
Contributor Author

@perrotta Perfectly right, in principle we could:

  • do the backport
  • implement it in DQM machines (after their tests)
  • in case of problem I can close the backport PR and do another one in due time

@fabiocos
Copy link
Contributor

@nminafra @perrotta @slava77 the event content of wf 10824 is indeed changed adding the collection, but I cannot see it in the Express configuration produced by Configuration/DataProcessing. This is likely not an issue with the PR, I will open a separate issue to follow up on that and merge this code for test.

@fabiocos
Copy link
Contributor

+1

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