Updates for new HGCal SiPM channel mapping#48571
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48571/45533
|
|
A new Pull Request was created by @kerstinlovisa for master. It involves the following packages:
@Dr15Jones, @Moanwar, @bsunanda, @civanch, @cmsbuild, @kpedro88, @makortel, @mdhildreth, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
assign hgcal-dpg |
|
New categories assigned: hgcal-dpg @cseez,@felicepantaleo,@pfs,@rovere you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
+1 Size: This PR adds an extra 36KB to repository Comparison SummarySummary:
|
|
Hi all, is anything else needed for this PR? Sorry to push, but we need these changes to map and index SiPM-on-tiles on the cassette that will be tested from September. |
|
Hello, is anything else needed for this PR or can it be merged? |
|
+Upgrade |
|
There are two comments I'm not sure if they were addressed |
| if (matched) { | ||
| wtypecode = sipm_typecode_match[0].str(); // assign sipm typecode as wafer type | ||
| } else { | ||
| edm::LogWarning("HGCalMappingIndexESSource") |
There was a problem hiding this comment.
maybe we should throw an exception no?
There was a problem hiding this comment.
I changed the warning to an exception.
There was a problem hiding this comment.
this is a test script, not sure if the bot will test it, but just in case can you run
python -m mypy scripts/make_sipm_channelmap.py
to look for obvious syntax errors? If you can use also some annotations, and add a short comment below each method , e.g.
def get_sequence_for_channel(channel : int, isHD : bool) -> int
"""this method returns the readout sequence for a given channel in a tileboard.
density is specified by the isHD parameter"""for others that may need to use this script eventually later
There was a problem hiding this comment.
this is a test script, not sure if the bot will test it,
The bot (or scram b runtests) will not run the script unless there is a test defined in ../test/BuildFile.xml that uses the script.
There was a problem hiding this comment.
This code is not really a test, it is the script I used to create the channel map text file that is now in cms-data. I included annotations and comments to the code and I ran the command python -m mypy scripts/make_sipm_channelmap.py which gave this output:
Success: no issues found in 1 source file
kerstinlovisa
force-pushed
the
hgcalmapping_SiPMupdatePR
branch
from
26bb44b to
b1d2732
Compare
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48571/45842
|
|
Pull request #48571 was updated. @Dr15Jones, @Moanwar, @bsunanda, @civanch, @cmsbuild, @cseez, @felicepantaleo, @kpedro88, @makortel, @mdhildreth, @pfs, @rovere, @srimanob, @subirsarkar can you please check and sign again. |
|
+geometry |
|
+1 |
|
please test |
cmsbuild
added
tests-started
hgcal-dpg-approved
and removed
tests-pending
hgcal-dpg-pending
labels
|
+1 Size: This PR adds an extra 24KB to repository Comparison SummarySummary:
|
|
Hi @cms-sw/upgrade-l2 & @cms-sw/orp-l2, can you have a look and sign off, please? All tests past, and we believe this PR is ready for release. Apologies for the constant badgering, but it would really facilitate cassette's testing & test beams at Fermilab and CERN next week. :) |
|
+Upgrade |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
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 will now be reviewed by the release team before it's merged. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
9 participants
PR description:
The channel mapping of the SiPM tileboards is being updated, which requires a few updates in the logic of reading the new channel map with:
The new channel mapping logic was presented during the DPG session of the last HGCal Workshop here.
This PR is made together with another PR to cms-data/Geometry-HGCalMapping: 5
PR validation:
The channel mapping was tested with Geometry/HGCalMapping/test/HGCalMappingESSourceTester.cc, and the mapping has been validated with DQM plots for low-density tileboards.
Validation of known input pixel arts was done 2 times, first with a known issue in the input (making the output misspelled with correct mapping):
Secondly with fixed error in the input and more variation in ADC input values for even better validation:
(Note that rotation of SiPM tileboards is not yet implemented in DQM)
@pfs @jniedzie @jalimena