ME0 pseudo digitizer update #18113

nickmccoll · 2017-03-29T01:36:42Z

We will not have the realistic digitizer in time for the Muon TDR samples, so we have decided to import the pseudo digitizer (replacing the old one) used for granularity studies. Please see for a presentation on its development:

https://indico.cern.ch/event/594656/contributions/2403265/attachments/1390281/2117550/nmccoll_12_16_16_ME0Newsigis.pdf#search=Mc%20Coll

While it is useful for granularity/neutron/# of layer studies, I added in a feature to simply use the granularity as defined by the geometry. This allows for the digitization, and merging, of hits as expected for the ME0 granularity. It also adds a new collection, which is simply a map of integers...pointing from input digi index to new digi index. Since we filter and merge inputs, it is a many to one map with empty entries.

(moved from: #18112)

cmsbuild · 2017-03-29T01:36:59Z

A new Pull Request was created by @nickmccoll (Nick McColl) for master.

It involves the following packages:

DataFormats/GEMDigi
SimMuon/GEMDigitizer

@cmsbuild, @civanch, @mdhildreth, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks.
@jhgoh this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

civanch · 2017-03-29T08:57:18Z

please test

cmsbuild · 2017-03-29T08:59:00Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18774/console Started: 2017/03/29 10:59

kpedro88 · 2017-03-29T09:46:05Z

SimMuon/GEMDigitizer/interface/ME0ReDigiProducer.h

@@ -12,18 +12,47 @@
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
 #include "FWCore/Framework/interface/ConsumesCollector.h"
 #include "DataFormats/GEMDigi/interface/ME0DigiPreRecoCollection.h"
-
+//#include "DataFormats/GeometryVector/interface/LocalPoint.h"


remove commented code

kpedro88 · 2017-03-29T09:47:01Z

SimMuon/GEMDigitizer/interface/ME0ReDigiProducer.h

+
+  //paramters
+  bool         useBuiltinGeo     ; //Use CMSSW defined geometry for digitization, not custom strips and partitions
+  unsigned int numberOfSrips     ; // Custom number of strips per partition


fix typo Srips -> Strips

kpedro88 · 2017-03-29T09:47:47Z

SimMuon/GEMDigitizer/python/muonME0ReDigis_cfi.py

-)
+    inputCollection    =cms.string('simMuonME0Digis'),
+    useBuiltinGeo      =cms.bool(True),   #Use CMSSW defined geometry for digitization, not custom strips and paritions
+    numberOfSrips      =cms.uint32(384), # If use custom: number of strips per partition                                             


fix typo Srips -> Strips

kpedro88 · 2017-03-29T09:48:40Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+	if(!mainLayer->nEtaPartitions())
+		throw cms::Exception("Setup") << "ME0ReDigiProducer::TemporaryGeometry::TemporaryGeometry() - ME0Layer has no partitions.";
+	if(mainLayer->nEtaPartitions() != 1)
+		throw cms::Exception("Setup") << "ME0ReDigiProducer::TemporaryGeometry::TemporaryGeometry() - This module is only compatitble with geometries that contain only one partiton per ME0Layer.";


Fix typo partiton -> partition

kpedro88 · 2017-03-29T09:50:21Z

SimMuon/GEMDigitizer/interface/ME0ReDigiProducer.h

+		float middleDistanceFromBeam;                        // radiusOfMainPartitionInCenter;
+		std::vector<TrapezoidalStripTopology * > stripTopos; // vector of Topos, one for each part
+		std::vector<std::vector<double> > tofs ;             //TOF to center of the partition:  [layer][part]
+		std::vector<float> partionTops;                      //Top of each position in the chamber's local coords


Fix typos partion, position -> partition (?)

kpedro88 · 2017-03-29T09:56:08Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+	partionTops.reserve(numberOfPartitions);
+	for(unsigned int iP = 0; iP < numberOfPartitions; ++iP){
+		double eta = (etaTop -etaBottom)*double(iP + 1)/double(numberOfPartitions) + etaBottom;
+		double distFromBeam = std::fabs(2 * zBottom /(std::exp(-1*eta) - std::exp(eta)  ));


Might be better to use std::sinh() here

kpedro88 · 2017-03-29T09:58:49Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+	double etaBottom  = globalBottom.eta();
+	double zBottom    = globalBottom.z();
+	partionTops.reserve(numberOfPartitions);
+	for(unsigned int iP = 0; iP < numberOfPartitions; ++iP){


I think this loop could be combined with the next loop

kpedro88 · 2017-03-29T10:13:42Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+		for(unsigned int iP = 0; iP < numberOfPartitions; ++iP){
+			const LocalPoint partCenter(0., getPartCenter(iP), 0.);
+			const GlobalPoint centralGP(mainChamber->layers()[iL]->etaPartitions()[0]->toGlobal(partCenter));
+			tofs[iL][iP] = (centralGP.mag() / 29.9792); //speed of light [cm/ns]


Better to use CLHEP::c_light/CLHEP::cm (units of CLHEP::c_light are mm/ns)

kpedro88 · 2017-03-29T10:23:39Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+		<< "Done building temporary geometry!" << std::endl;
+
+		if(tempGeo->numLayers() != layerReadout.size() )
+			throw cms::Exception("Configuration") << "ME0ReDigiProducer::beginRun() - The geoemtry has "<<tempGeo->numLayers()


Fix typo geoemtry -> geometry

kpedro88 · 2017-03-29T10:24:59Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

 {
-  /*


Are these deleted comments no longer correct? It would be nice to replace them with an updated version, if so.

kpedro88 · 2017-03-29T10:26:13Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

-	if (newRoll < ME0DetId::minRollId || newRoll > ME0DetId::maxRollId){
-	  newRoll = detId.roll();
+
+	LogDebug("ME0ReDigiProducer::buildDigis") << "Begin bulding digis."<<std::endl;


Fix typo bulding -> building

kpedro88 · 2017-03-29T10:27:15Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+		for(unsigned int iP = 0; iP < nPartitions; ++iP){
+			const unsigned int mapPartIDX = layer->etaPartitions()[iP]->id().roll() -1;
+			const GlobalPoint centralGP(layer->etaPartitions()[iP]->position());
+			tofs[mapLayIDX][mapPartIDX] = (centralGP.mag() / 29.9792); //speed of light [cm/ns]


Better to use CLHEP::c_light/CLHEP::cm (units of CLHEP::c_light are mm/ns)

kpedro88 · 2017-03-29T10:29:28Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+			}
+
+			//filter if outside of readout window
+			const int bxIdx = std::round(tof/25.0);


Ideally "25" would be a constant member of the class (here and elsewhere, e.g. a few lines below)

kpedro88 · 2017-03-29T10:30:23Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+
+	//get digitized location
+	LocalPoint  digiPartLocalPoint = topo->localPosition(stripF);
+	digiLocalError = topo->localError(stripF, 1./sqrt(12.));


Explanation of magic number 12?

that is the usual way to compute the resolution, dividing the strip pitch by sqrt(12)...

The std dev. of a flat distribution with length L is L/sqrt(12). The strip topology expects the error in units of strips. I added a comment.

kpedro88 · 2017-03-29T10:31:47Z

-1
pending responses to code review

calabria · 2017-04-01T14:13:37Z

can we please integrate this PR?

davidlange6 · 2017-04-01T16:56:06Z

SimMuon/GEMDigitizer/src/ME0ReDigiProducer.cc

+		LogTrace("ME0ReDigiProducer::buildDigis") << "Starting with roll: "<< me0Id<<std::endl;
+
+		//setup map for this chamber/eta partition
+		ChamberDigiMap chDigiMap;


@nickmccoll - it looks like a std::vectorstd::tuple or at worst a std::mapstd::tuple,... would serve the purpose of this map of map of maps (!!) without all the memory problems.

@davidlange6 I changed it to a map of a tuple. I think it that the map is better than a vector in this case because when we create each digi we have to check if it has already been created.

…mssw into ME0PseudoDigiUpdate

cmsbuild · 2017-04-01T22:05:10Z

Pull request #18113 was updated. @cmsbuild, @civanch, @mdhildreth, @kpedro88, @davidlange6 can you please check and sign again.

davidlange6 · 2017-04-02T07:27:23Z

please test

cmsbuild · 2017-04-02T07:27:45Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18853/console Started: 2017/04/02 09:29

cmsbuild · 2017-04-02T09:09:05Z

+1
Tested at: 0fc1826
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18113/18853/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
0fc1826
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18113/18853/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18113/18853/git-merge-result

cmsbuild · 2017-04-02T09:09:07Z

Comparison job queued.

cmsbuild · 2017-04-02T12:18:21Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18113/18853/summary.html

nickmccoll added 3 commits March 28, 2017 13:54

ME0DigiPreRecoMap for matching original pseudo digis to re-digis

bceedd3

Imported Re-Digitizer with variable geometry

f38b5e6

Updated code to digitize following the built-in geometry

3dd176d

cmsbuild added this to the Next CMSSW_9_1_X milestone Mar 29, 2017

cmsbuild added comparison-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Mar 29, 2017

cmsbuild added tests-started and removed tests-pending labels Mar 29, 2017

kpedro88 reviewed Mar 29, 2017

View reviewed changes

cmsbuild added the simulation-approved label Mar 31, 2017

davidlange6 reviewed Apr 1, 2017

View reviewed changes

nickmccoll added 2 commits April 1, 2017 15:03

Changed container to map of a tuple

6887647

Merge branch 'ME0PseudoDigiUpdate' of https://github.com/nickmccoll/c…

0fc1826

…mssw into ME0PseudoDigiUpdate

cmsbuild added comparison-pending pending-signatures simulation-pending tests-pending upgrade-pending and removed comparison-available fully-signed simulation-approved tests-approved upgrade-approved labels Apr 1, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 2, 2017

cmsbuild added tests-approved and removed tests-started labels Apr 2, 2017

davidlange6 merged commit 04fee3d into cms-sw:master Apr 2, 2017

cmsbuild added comparison-available and removed comparison-pending labels Apr 2, 2017

perrotta mentioned this pull request Apr 4, 2017

ME0Segment update for muon tdr samples #18127

Merged

nickmccoll mentioned this pull request Apr 11, 2017

By default digitize single partition ME0 geometry with custom parameters #18302

Merged

ME0 pseudo digitizer update #18113

ME0 pseudo digitizer update #18113

Conversation

nickmccoll commented Mar 29, 2017

cmsbuild commented Mar 29, 2017

civanch commented Mar 29, 2017

cmsbuild commented Mar 29, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpedro88 commented Mar 29, 2017

calabria commented Apr 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Apr 1, 2017

davidlange6 commented Apr 2, 2017

cmsbuild commented Apr 2, 2017 • edited

cmsbuild commented Apr 2, 2017

cmsbuild commented Apr 2, 2017

cmsbuild commented Apr 2, 2017

cmsbuild commented Mar 29, 2017 •

edited

cmsbuild commented Apr 2, 2017 •

edited