Software Design Update for the Phase2 Tracker Digitizer #30582
Conversation
…n the framework. Phase2Digitizer configuraion updated to to have a common PSet for Pixel and individual IT-pixel algotithms can clone it and make necessary modifications. A bug fix has been made in the dual slope correction to avoid a shift of 0.5 ToT bin
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30582/16819
|
|
A new Pull Request was created by @suchandradutta (Suchandra Dutta) for master. It involves the following packages: SimTracker/SiPhase2Digitizer @cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins.
|
| namespace { | ||
| double calcQ(float x) { | ||
| auto xx = std::min(0.5f * x * x, 12.5f); | ||
| return 0.5 * (1.0 - std::copysign(std::sqrt(1.f - unsafe_expf<4>(-xx * (1.f + 0.2733f / (1.f + 0.147f * xx)))), x)); |
There was a problem hiding this comment.
use named constants rather than magic numbers, or at least provide a comment explaining what the numbers mean
| double signalScale = 1.0; | ||
| // fill collection_points for this SimHit, indpendent of topology | ||
| if (select_hit(hit, (pixdet->surface().toGlobal(hit.localPosition()).mag() * c_inv), signalScale)) { | ||
| const auto ionization_points = primary_ionization(hit); // fills ionization_points |
There was a problem hiding this comment.
const auto& would be better (captures temporary object returned by the function and extends its lifetime to match the current scope, rather than making a copy)
| const auto ionization_points = primary_ionization(hit); // fills ionization_points | ||
|
|
||
| // transforms ionization_points -> collection_points | ||
| const auto collection_points = drift(hit, pixdet, bfield, ionization_points); |
| elossVector = fluctuateEloss(hit.particleType(), hit.pabs(), eLoss, length, NumberOfSegments); | ||
| } else { | ||
| float averageEloss = eLoss / NumberOfSegments; | ||
| std::fill(std::begin(elossVector), std::end(elossVector), averageEloss); |
There was a problem hiding this comment.
reserve() doesn't actually create entries, so I don't think this fill() will work. Rather:
elossVector.resize(NumberOfSegments, averageEloss);
| // material,*, momentum,energy,*, *, mass | ||
| //myglandz_(14.,segmentLength,2.,2.,dedx,de,0.14); | ||
| // material,*, momentum,energy,*, *, mass | ||
| // myglandz_(14.,segmentLength,2.,2.,dedx,de,0.14); |
There was a problem hiding this comment.
is this commented-out code still useful? if not, delete it
|
+1 |
|
Comparison job queued. |
| @@ -258,11 +221,6 @@ | |||
| AddInefficiency = False, | |||
| AddThresholdSmearing = False, | |||
| ), | |||
| Pixel3DDigitizerAlgorithm = dict( | |||
There was a problem hiding this comment.
@suchandradutta,
This reverts f38453e from @duartej. Please restore it as it was previously.
|
Comparison is ready Comparison Summary:
|
I have checked the Phase2Digi DQM histograms after the change in the return type double calcQ(float x) to float calcQ(float x) as suggested here and I can see the minor changes which is seen here |
|
+upgrade |
|
+1 |
cmsbuild
added
fully-signed
simulation-approved
and removed
pending-signatures
simulation-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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
please test |
|
The tests are being triggered in jenkins.
|
cmsbuild
added
comparison-pending
tests-pending
tests-started
and removed
comparison-available
tests-approved
tests-pending
labels
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
merge |
PR description:
Software Design Update for the Phase2 Tracker Digitizer
The interface of the base algorithm class Phase2TrackerDigitizerAlgorithm has been updated to support Pixel3DDigitizerAlgorithm properly in the framework. Prototype of a number of methods of Phase2TrackerDigitizerAlgorithm have changed. Code duplication removed. Integration of new algorithms, if any, will be more straight-forward now.
Phase2TrackerDigitizer configuration updated to have a common PSet for Pixel. Individual IT-pixel algorithms clone it with necessary modifications
A bug fix has been made in the dual slope correction to avoid a shift of 0.5 ToT bin
A logical bug was fixed in Phase2TrackerDigitizerAlgorithm::primary_ionization(...)
what changes are expected in the output if any --> none
what other PRs or externals it depends upon if any --> none
link to any additional material useful to provide a documentation for this PR (slides, JIRA tickets, related pull requestes, hypernews, TWiki or Indico pages) --> https://indico.cern.ch/event/879046/contributions/3931489/attachments/2068895/3472721/SWDesignUpdate_validation_03072020.pdf
PR validation:
scram b runtests
runTheMatrix.py -l limited -i all --ibeos
if this PR is a backport please specify the original PR and why you need to backport that PR:
Not applicable
@subirsarkar @emiglior @skinnari @duartej @OzAmram