csc bad channels handling in rechit building updated for unganged me11a #5713
Conversation
|
A new Pull Request was created by @ptcox for CMSSW_7_3_X. csc bad channels handling in rechit building updated for unganged me11a It involves the following packages: CalibMuon/CSCCalibration @nclopezo, @cerminar, @cmsbuild, @diguida, @rcastello, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks. |
|
@cmsbuild please test |
|
+1 |
…it_building_updated_for_unganged_me11a csc bad channels handling in rechit building updated for unganged me11a
|
Hi Salvatore, No, this alternative would not require new payloads. The existing indexing would be unchanged, and the payload content also. The situation is a little confusing because of the split strip plane in the ME1/1 CSCs. Like all CSCs, an ME1/1 chamber contains 6 layers of gas, separated by 6 layers of anode wires, and 6 layers of strips. But the strip plane is split into two independently read-out regions at eta = 2.1. The high-eta region is ME1/1A and the low eta region ME1/1B. The two regions are machined into different numbers of strips too - 64 in B and 48 in A. Offline, for many years, we have treated ME1/1A and ME1/1B as two separate virtual chambers, which share a wire plane (and some wires cross both A and B strip regions because the wires are tilted in ME1/1 to compensate for the Lorentz drift due to the higher magnetic field in the ME1/1 region). Thus offline we have separate CSCDetId's to label ME1/1A and ME1/1B. We do this by arbitrarily defining ring=1 to mean ME1/1B and ring=4 to mean ME1/1A. It is these virtual chambers that get aligned and have local coordinates that can be transformed to and from CMS global coordinates. However, online that distinction was unnecessary. All we have in raw data is stuff coming out on readout channels. ME1/1B has 64 channels, and in the past the 48 strips of ME1/1A were ganged to 16 channels so that ME1/1 as a whole had 80 channels. This is the same number as in almost all other CSCs. The result of this is that for 'bad channels' flagging I could use one bitset<80> for one entire layer of a ME1/1 CSC: 1-64 for A, 65-80 for B. To extend it for unganged ME1/1A, there are two alternatives: 1) extend the bitset to 64+48=112, or 2) keep 80 and add extra bad channels bitsets for each ME1/1A layer. Option 2) is sort of equivalent to what we have done for all the other conditions data - the extra ME1/1A information is appended to the previous payloads. Unfortunately, for the bad channels handling it would require setting up a NEW indexing, by 'virtual' layers in the system, rather than the existing indexing by 'real' (physical) layer. That is the where significant additions to the indexing classes would be required. Option 1) is straightforward - the existing indexing still works, but allows flagging future bad channels in the extended ME1/1A list 1-48 == 65-112 rather than the old 1-16 == 65-80. However, in the past I was filling a vector< bitset<80> > for all 3240 layers once per read of the conditions data, and holding this vector throughout the event processing. I decided it was probably more sensible to release that use of memory in favour of more computation, which is in any case trivial on the scale of CMSSW processing, and just fill the bitset, now a bitset<112>, for the specific layer in which one is reconstructing rechits. Thus instead of filling a vector once, I avoid the vector store, and instead at the start of rechit building in a layer go back to the conditions, unpack the bad channels info again and fill the bitset just for that layer. So this is a less memory-use but more computation. Hope this helps clarify the situation. Regards, Tim P.S. Please be careful of '112'. The maximum number of wire groups (wires are ganged in the CSCs and read out by wiregroup) in any CSC type is '112', as it always has been. Pre-LS1, the maximum number of strips channels in any CSC was 80. Post-LS1 it becomes 64+48 = 112 in ME1/1. The fact that both are 112 is pure coincidence! Also, I'd really prefer to keep 112 explicit. In this case, adding a named const does not help clarify the situation to anyone who is a CSC expert - it adds a layer of 'hiding' that hinders understanding. On Oct 9, 2014, at 11:09, Salvatore Di Guida notifications@github.com wrote:
|
|
@ptcox IMHO, this clarifies the situation to anyone (being or not a CSC expert), as the meaning of each quantity is explicitly mapped to a name of a constant expression and the subsequent usage; besides this, it also improves code readability and maintainability, as it removes the usage of "magic numbers". |
|
I cannot object, formally, to replacing the 112's. But I still don't like it :) One thing I dislike about it is that one always has to be very careful about whether one is dealing with physical strips or strip readout channels and these concepts are scattered throughout the code, in different contexts. Originally I put a lot of effort into encapsulating this sort of information in the Geometry classes, which seemed reasonable at the time. But then people came along and added another const here and there, just where they needed it (like now), rather than getting them from one predefined location. So really one should go through all the code and sort that out. I do not have the time or enthusiasm to do it right now. I don't really understand your comment about 'follow up with Muon Alignment'. There is nothing to follow up with Muon Alignment. I was just stating the formal CMS software model: a unit of a detector is represented by a Det (or DetUnit), with a DetId, and for tracking detectors one of the Det's functions is to handle coordinate transformations between local and global coordinates. A Det is initially positioned in IDEAL global coordinates, and what alignment effectively does is 'corrects' these coordinates. But this is transparent to the outside world: internally the coordinate transformations just get adjusted so that the local<->global transformations involve 'REAL' global coordinates rather than 'IDEAL' global coordinates. For the CSCs, the Det modelling is the same as always - nothing has changed or needs to be changed. Strips and Wires are NOT alignable objects. What IS aligned are the chamber frames - the 'box' containing one chamber, which also contains two 'alignment pins', two fixed points that project to the outer surface, and which are also used as references when assembling the machined strip and wire planes into one chamber. All wire and strip geometry in the software is referenced to these alignment pins. The alignment procedure itself knows nothing explicitly about wires and strips. Regards, Tim On Oct 9, 2014, at 12:44, Salvatore Di Guida notifications@github.com wrote:
|
|
@ptcox |
[Directed at 73X]. This updates CSC bad channel handling in local reco to allow extension to unganged ME11A channels. Amazingly tricky to do this because the extra channels are appended to the bad channels conditions data. At the same time I decided it was more sensible to change the way the bad channels data are handled. Until now, each time the conditions data changes, the bad channels information was unpacked and stored in a large vector - one 80-bit bitset for the 80 strip channels of each layer in the CSC system: there are 3240 layers in the system. There is also a 112-bitset for the possible bad wiregroups in each layer. However, with unganged ME11A instead of 80 strip channels, we'd have to increase to 64+48=112 bits for the bad strip channels too. So I've changed the approach so the bitsets for a given layer are filled only when that layer is being reconstructed. This saves on storage at the expense of computing time, which is basically trivial. This change is made in CSCConditions, a class in CalibMuon/CSCCalibration, which is our interface to CSC conditions data. There are NO changes to actual conditions data handling - just how the local reco accesses it once it is available. This is done in RecoLocalMuon/CSCRecHitD, where rechits are built. CSCConditions is also used in SimMuon/CSCDigitizer, but the bad channels part is NOT used there (since we don't know how to simulate bad channels even if we know somewhere in the complex change from detector element to read-out signal something is broken.) So there is NO change to Sim code. I added a new test analyzer for the bad channels handling in CSCRecHitD, called CSCRecoBadChannelsAnalyzer. This has a new config in CSCRecHitD/test. I also updated the sample config file for the rechit builder to 72X/73X level. This is run_on_raw_72X.py also in CSCRecHitD/test. I have run on 2500 events from a real data 2012 SingleMu raw relval sample and dumped all rechits built and find identical output for the new code and the old code. I have also run runTheMatrix.py -s --useInput all and this completes with no errors.