New sipm model 810preX #15058

pdudero · 2016-06-29T16:10:02Z

No description provided.

cmsbuild · 2016-06-29T16:10:23Z

A new Pull Request was created by @pdudero for CMSSW_8_1_X.

It involves the following packages:

CalibCalorimetry/HcalAlgos
CalibCalorimetry/HcalPlugins
SimCalorimetry/HcalSimAlgos
SimCalorimetry/HcalSimProducers
SimGeneral/MixingModule

@ghellwig, @civanch, @cerminar, @cmsbuild, @franzoni, @mdhildreth, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @tocheng, @wmtan this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

civanch · 2016-06-29T16:14:55Z

@pdudero , please, provide description.

pdudero · 2016-06-29T16:16:08Z

This is updated Hcal SiPM simulation code for 81X.

On 6/29/2016 11:14 AM, Vladimir Ivantchenko wrote:

@pdudero https://github.com/pdudero , please, provide description.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15058 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AExICDGqGU6aTwTNy8u1E4uEbFpdm7Vtks5qQpoCgaJpZM4JBThX.

kpedro88 · 2016-06-29T16:16:59Z

This PR updates the SiPM simulation. Changes:

Dark current noise is generated separately from pedestal noise
Crosstalk is modeled using a Borel distribution (see: http://arxiv.org/abs/1302.1455, https://arxiv.org/abs/1109.2014)
Pulse shape is modeled by a sum of lognormal distributions

The parameters selected for the simulation are based on test beam data, though they may need to be updated in the future. (Some of the parameters might also be moved into the database to be set on a per-channel basis, but this is still under discussion.)

Phil, can you post a plot of the noise distribution using the simulation as it is in this PR?

attn: @abdoulline, @bsunanda, @jmmans, @pastika

kpedro88 · 2016-06-29T16:17:09Z

@cmsbuild please test

cmsbuild · 2016-06-29T16:17:29Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13774/console

cmsbuild · 2016-06-29T16:26:43Z

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

cmsbuild · 2016-06-29T16:26:55Z

-1

Tested at: 4260708

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15058/13774/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

Build:

I found an error when building:

>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/Utilities/StaticAnalyzers/src/EDMPluginDumper.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/Utilities/StaticAnalyzers/src/FiniteMathChecker.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/Utilities/StaticAnalyzers/src/UsingNamespace.cpp 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/Utilities/StaticAnalyzers/src/FunctionDumper.cpp 
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc: In constructor 'HcalHardcodeCalibrations::HcalHardcodeCalibrations(const edm::ParameterSet&)':
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc:143:16: error: 'class HcalDbHardcode' has no member named 'setLumi'
     dbHardcode.setLumi(iLumi);
                ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc:144:16: error: 'class HcalDbHardcode' has no member named 'setLumiOffset'
     dbHardcode.setLumiOffset(iConfig.getParameter("iLumiOffset"));
                ^

Clang:

I found a compilation error while trying to compile with clang:
I used this command:
scram b vclean && scram build -k -j 16 USER_CXXFLAGS='-fsyntax-only' COMPILER='llvm compile'

>> Compiling edm plugin /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/SimCalorimetry/HcalSimAlgos/test/HcalDigitizerTest.cc 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/SimCalorimetry/HcalSimAlgos/test/HcalTimeSlewTest.cpp 
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/SimCalorimetry/HcalSimAlgos/test/HcalShapeIntegratorTest.cpp 
1 warning generated.
>> Compiling  /build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/SimCalorimetry/HcalSimAlgos/test/HPDIonFeedbackTest.cpp 
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc:143:16: error: no member named 'setLumi' in 'HcalDbHardcode'
    dbHardcode.setLumi(iLumi);
    ~~~~~~~~~~ ^
/build/cmsbuild/jenkins-workarea/workspace/ib-any-integration/CMSSW_8_1_X_2016-06-28-2300/src/CalibCalorimetry/HcalPlugins/src/HcalHardcodeCalibrations.cc:144:16: error: no member named 'setLumiOffset' in 'HcalDbHardcode'
    dbHardcode.setLumiOffset(iConfig.getParameter("iLumiOffset"));
    ~~~~~~~~~~ ^

kpedro88 · 2016-06-29T16:27:50Z

@pdudero needs some more cleanup in HcalHardcodeCalibrations...

cmsbuild · 2016-06-29T17:00:06Z

Pull request #15058 was updated. @ghellwig, @civanch, @cerminar, @cmsbuild, @franzoni, @mdhildreth, @mmusich, @davidlange6 can you please check and sign again.

kpedro88 · 2016-06-29T17:01:33Z

@cmsbuild please test

cmsbuild · 2016-06-29T17:01:51Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/13778/console

cmsbuild · 2016-07-01T20:02:26Z

+1
Tested at: d84254e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15058/13853/summary.html

cmsbuild · 2016-07-01T21:47:28Z

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

The workflows 140.53, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

civanch · 2016-07-03T16:45:49Z

@pdudero , there are few places where math fuctions intensively used: HcalSiPM::Borel, HcalSiPMHitResponse::Y11TimeResponse.

It seems that if you substitute exp(x)_pow(t,y) by exp(x + log(t)_y) the code will be faster. Also if the code inside HcalDbHadrcode.cc is used very frequently I would replace computational of exp, log of integer agruments by a lookup table (one solution exist within Geant4 but here the library cannot depend on it, however, similar utility class may be provided for CMSSW).

kpedro88 · 2016-07-03T23:21:34Z

@civanch

The results of HcalSiPM::Borel() are cached in a hash table to reduce the number of computations:
https://github.com/pdudero/cmssw/blob/d84254eb3e77295c102aded6badde0cc91dbe1fa/SimCalorimetry/HcalSimAlgos/src/HcalSiPM.cc#L74
but the change you suggest could still be useful. @pdudero, can you implement this and make sure it gives the same result? (And also in the HcalSiPMHitRespones::Y11TimePDF() function, though it should be noted we haven't actually changed this function.)

I don't see any use of exp or log in HcalDbHardcode or HcalHardcodeCalibrations...

pdudero · 2016-07-04T16:41:32Z

@civanch, @kpedro88,
Is log really that much faster than pow? Regardless, I am not inclined to change HcalSiPM::Borel for the following reasons:

As Kevin pointed out, the hash table is cached.
You will note that exp(X)*pow(t,Y) = (exp(X + log(t)Y) is used for values (n-k)>=100.
Values below 100 (which are the most commonly used, for channels with only noise) use the precise, canonical formulation for the Borel distribution and ALL are calculated once upon initialization, which is when the crosstalk parameter is set; see
https://github.com/pdudero/cmssw/blob/newSiPMmodel810preX/SimCalorimetry/HcalSimAlgos/src/HcalSiPM.cc#L154-L157

civanch · 2016-07-05T12:41:16Z

@pdudero, concerning pow - I would guess it is equivalent exp+log, this seeseems not an issue now. Concerning pointers - have you use following signature:
std::unique_ptr uptr(new my class);
uptr.get()->my method();

kpedro88 · 2016-07-06T00:29:03Z

@civanch as far as I can tell, the only pointer used anywhere in this PR is CLHEP::HepRandomEngine* engine, and that is clearly not managed by any of this code. Did you have any specific places where code should be changed?

civanch · 2016-07-06T07:24:57Z

+1
after recent PR updates I cannot find out the place any more..

kpedro88 · 2016-07-06T22:36:21Z

@mmusich can you take a quick look? In your domain, just some minor changes in HcalHardcode stuff

abdoulline · 2016-07-07T06:01:37Z

@mmusich
Marco, after all the issues fixed and corrections made, I believe this PR deserves to be approved to help us to prepare a realistic MC for 2017.

mmusich · 2016-07-07T07:21:29Z

+1

cmsbuild · 2016-07-07T07:21:50Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

bsunanda · 2016-07-07T16:54:41Z

@davidlange6 David could you integrate this at your earliest convenience so that we can start validating 2017 simulation chain?

davidlange6 · 2016-07-14T06:42:25Z

SimCalorimetry/HcalSimAlgos/src/HcalSiPM.cc

+  if (logb >= -20)  { // protect against underflow
+    b=(dk/dn);
+    if ((n-k)<100)
+      b *= (exp(-ldn)*pow(ldn,dnk))/TMath::Factorial(n-k);


@pdudero what is a typical value of dunk? (Eg most often 2-3 or much larger?

pdudero · 2016-07-14T14:56:13Z

@davidlange6 , I could talk about the MPV of the distribution, but I suspect that's not what you're asking. In terms of how the function is used, it isn't very useful to ask the question. The function is used to calculate lookup tables of the CDFs, the lowest 100 of them at initialization, and the rest on the fly. So for a given "n", "k" is scanned over a range determined by an epsilon cutoff. Then a value is chosen out of the table by throwing a trial from a flat random distribution over the CDF.

davidlange6 · 2016-07-15T14:41:14Z

+1
ok - that answers my question...thanks

pdudero added 4 commits June 28, 2016 14:57

new SiPM sim code for 810preX

a0f6d2a

deleted unused code

2cc0f41

python parameter changes for SiPMs

d4d5be0

python parameter changes for SiPMs

4260708

cmsbuild added this to the Next CMSSW_8_1_X milestone Jun 29, 2016

cmsbuild added simulation-pending alca-pending pending-signatures tests-pending orp-pending comparison-pending labels Jun 29, 2016

cmsbuild added tests-started and removed tests-pending labels Jun 29, 2016

cmsbuild added tests-rejected and removed tests-started labels Jun 29, 2016

clean up SiPM Lumi dependence

8d399de

cmsbuild added tests-pending and removed tests-rejected labels Jun 29, 2016

cmsbuild added tests-approved and removed tests-started labels Jul 1, 2016

cmsbuild added comparison-available and removed comparison-pending labels Jul 1, 2016

cmsbuild added simulation-approved and removed simulation-pending labels Jul 6, 2016

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Jul 7, 2016

davidlange6 reviewed Jul 14, 2016
View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Jul 15, 2016

cmsbuild merged commit dfd089d into cms-sw:CMSSW_8_1_X Jul 15, 2016

kpedro88 mentioned this pull request Oct 22, 2016

SiPM nonlinearity, parameters from CondDB, update tau, cleanup #16315

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New sipm model 810preX #15058

New sipm model 810preX #15058

pdudero commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

civanch commented Jun 29, 2016

pdudero commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016 •

edited

cmsbuild commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016 •

edited

cmsbuild commented Jul 1, 2016

cmsbuild commented Jul 1, 2016

civanch commented Jul 3, 2016

kpedro88 commented Jul 3, 2016

pdudero commented Jul 4, 2016 •

edited

civanch commented Jul 5, 2016

kpedro88 commented Jul 6, 2016

civanch commented Jul 6, 2016

kpedro88 commented Jul 6, 2016

abdoulline commented Jul 7, 2016

mmusich commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

bsunanda commented Jul 7, 2016

davidlange6 Jul 14, 2016

pdudero commented Jul 14, 2016 •

edited

davidlange6 commented Jul 15, 2016

New sipm model 810preX #15058

New sipm model 810preX #15058

Conversation

pdudero commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

civanch commented Jun 29, 2016

pdudero commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016 • edited

cmsbuild commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016

kpedro88 commented Jun 29, 2016

cmsbuild commented Jun 29, 2016 • edited

cmsbuild commented Jul 1, 2016

cmsbuild commented Jul 1, 2016

civanch commented Jul 3, 2016

kpedro88 commented Jul 3, 2016

pdudero commented Jul 4, 2016 • edited

civanch commented Jul 5, 2016

kpedro88 commented Jul 6, 2016

civanch commented Jul 6, 2016

kpedro88 commented Jul 6, 2016

abdoulline commented Jul 7, 2016

mmusich commented Jul 7, 2016

cmsbuild commented Jul 7, 2016

bsunanda commented Jul 7, 2016

davidlange6 Jul 14, 2016

Choose a reason for hiding this comment

pdudero commented Jul 14, 2016 • edited

davidlange6 commented Jul 15, 2016

cmsbuild commented Jun 29, 2016 •

edited

cmsbuild commented Jun 29, 2016 •

edited

pdudero commented Jul 4, 2016 •

edited

pdudero commented Jul 14, 2016 •

edited