CTPPS: diamond mapping for 2017 data taking #18490
Conversation
Conflicts: EventFilter/CTPPSRawToDigi/python/ctppsRawToDigi_cff.py
…gi object definition
|
A new Pull Request was created by @forthommel (Laurent Forthomme) for master. It involves the following packages: CondFormats/CTPPSReadoutObjects @perrotta, @ghellwig, @civanch, @arunhep, @cerminar, @cmsbuild, @franzoni, @mdhildreth, @slava77, @ggovi, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
On 4/27/17 1:42 AM, Laurent Forthomme wrote:
This PR introduces the channels mapping for 2017 diamond detectors
operations, along with a limited set of small changes:
is this required for data commissioning in May?
How early in May?
|
|
Hi @slava77 |
|
Comparison is ready Comparison Summary:
|
|
Pull request has been put on hold by @perrotta |
|
Hi @perrotta |
|
@forthommel : can you please have a look yourself? You know better than me where exactly to look at in order to check that the whole new mapping in the xml file of this PR is correct. Then you can post here that evidence (either plot or text output). Thank you! |
|
On 4/29/17 1:08 AM, Laurent Forthomme wrote:
Hi @perrotta <https://github.com/perrotta>
Indeed, the files you can find in this directory are the raw (binary)
streamer outputs from miniDAQ. You may use a |NewEventStreamFileReader|
source to access them, or use
this example configuration file
<https://github.com/cms-sw/cmssw/files/965992/ctpps_diamond_mapping_test_cfg.py.txt>
and look at the |ctppsDiamondRawToDigi| collection inside (no rechits
nor local tracks are expected, as these runs are produced without any
beam... fortunately!)
Or if you prefer I can produce a raw EDM file out of these |t0streamer|
binaries?
are there any files with charge injection?
so that there are hit payloads in the data frames.
If readout is not zero suppressed, then existing files are OK.
please clarify
Thank you.
… —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18490 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbq4AEQO_AbNMYWmLX6334Xn4ySHHks5r0u_kgaJpZM4NJ6Vp>.
|
|
Forwarding @nminafra's comment: we have a complex (and analog) cabling, hence the only way to check the mapping is using the strips and the beam. We have a set of "tomography plots" in our diamond DQM for this. Bottom line: we checked and double checked the mapping, in case of error it will not cause warnings or errors but, unfortunately, we could have some misplaced channels that will require a revised version (only) of the xml after the first data taking during the alignment run. As for your last comment, @slava77, we unfortunately do not have any charge injection mechanism on the HW side (but dark counts are negligibles), and zero suppression is not yet implemented in our FW. |
|
On 4/29/17 8:01 AM, Laurent Forthomme wrote:
Hi @slava77 <https://github.com/slava77>, @perrotta
<https://github.com/perrotta>,
Forwarding @nminafra <https://github.com/nminafra>'s comment: we have a
complex (and analog) cabling, hence the only way to check the mapping is
using the strips and the beam. We have a set of "tomography plots" in
our diamond DQM for this.
This cabling was carefully done, but unfortunately we have no way to
cross check without the beam. However, this will not produce any error
or warning, only the tracks will be affected.
But again all the digital part has been checked (incl. the channel id).
It is assigned by the digitizer board in the tunnel and is also written
in the xml.
If an id does not correspond to the correct position in the frame, the
unpacker should raise an error (and it is not happening in our tests).
Bottom line: we checked and double checked the mapping, in case of error
it will not cause warnings or errors but, unfortunately, we could have
some misplaced channels that will require a revised version (only) of
the xml after the first data taking during the alignment run.
As for your last comment, @slava77 <https://github.com/slava77>, we
unfortunately do not have any charge injection mechanism on the HW side
(but dark counts are negligibles), and zero suppression is not yet
implemented in our FW.
OK.
Just to be sure, have your tests on 2017 data been done with the code
directly from the PR?
If correctness of all logic can't be checked due to lack of non-zero inputs,
we should at least be sure that all relevant files are committed and run
OK even on these zeroed-out inputs.
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18490 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbjpcKtZwRmce6V2iSSVhKRsSTBBBks5r01DZgaJpZM4NJ6Vp>.
|
|
Thanks for this fast reaction! Yes, these tests were performed on top of this PR. |
|
@forthommel : I'm already fine with it. If also @slava77 is ok with your answers I will un-hold |
|
On 5/2/17 2:00 AM, perrotta wrote:
@forthommel <https://github.com/forthommel> : I'm already fine with it.
If also @slava77 <https://github.com/slava77> is ok with your answers I
will un-hold
fine with me
… —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18490 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbrKfPnhcZCAZn1wUD4B5Gf7pGb_uks5r1vCqgaJpZM4NJ6Vp>.
|
|
unhold |
| @@ -0,0 +1,129 @@ | |||
| <top> | |||
There was a problem hiding this comment.
hello @forthommel
are you planning on moving this mapping content to the conditions database ?
I guess you need CTPPSPixelDAQMappingRcd
to be provided to you from this parallel pr https://github.com/cms-sw/cmssw/pull/18359/files#diff-03e8ca1cea42e0df6e5deb53fc5dd4be ?
There was a problem hiding this comment.
we're looking at #18359 in parallel
There was a problem hiding this comment.
Hi @franzoni
As noted in #18490 (comment), this PR only tackles the diamond detectors part (and not the pixel detectors using this CTPPSPixelDAQMappingRcd record). Hence, a follow-up is indeed expected for the migration of this xml mapping to the conditions db!
|
in fact particularly if you anticipate, This needs a focussed discussion on tag integration and GT update |
|
+1 this PR is ok |
|
+1 |
cmsbuild
added
db-approved
fully-signed
and removed
db-pending
pending-signatures
labels
|
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
Many thanks for your reviews! |
|
+1 |
This PR introduces the channels mapping for 2017 diamond detectors operations, along with a limited set of small changes:
CTPPSDiamondDetIdobject is modified to account for a "more trivial" list of channels.ctppsDiamondRawToDigiobject definition.