Skip to content

redefine IT digitizer ToF window + add customize fcn for activating IT signal shape with RelVals#41273

Merged
cmsbuild merged 4 commits intocms-sw:masterfrom
emiglior:add_customize_ITSignalShape_cmssw_13_1_X
Apr 11, 2023
Merged

redefine IT digitizer ToF window + add customize fcn for activating IT signal shape with RelVals#41273
cmsbuild merged 4 commits intocms-sw:masterfrom
emiglior:add_customize_ITSignalShape_cmssw_13_1_X

Conversation

@emiglior
Copy link
Contributor

@emiglior emiglior commented Apr 4, 2023

PR description:

This PR concerns the phase-2 upgrade (Inner Tracker).

Motivation: The PR improves the description of the data rates in output from the Inner Tracker (IT) and it allows the central production of PU200 RelVals for the studies carried on by the phase-2 Tracker DPS group.

This PR updates the configuration of the PixelDigitizerAlgorithm and of the Pixel3DDigitizerAlgorithm used for the IT:

  1. it updates the [tofLowerCut, tofUpperCut] selection from [-12.5,+12.5] ns to a more sensible window [-5,+20] ns;
  2. it introduces a process modifier to activate the emulation of the signal shape of the IT readout chip (already in CMSSW, but OFF by default);
  3. it introduces the corresponding workflows with suffix xxxxx.141. The default workflows (xxxxx.0) have still the emulation of IT signal shape OFF.

The effect of the PR has been discussed in the Tracker Phase-2 simulation meeting:

  • motivation for the choice of the new tof window: slide 3 of this talk
  • effect on the occupancy of the new tof window or of the activation of the IT signal shape: slide 1 of this talk. On average, the effect on the occupancy (DIGI level) due to this change is expected to be limited.

PR validation:

The PR has been validated running the phase-2 w/f 25061.0 (reference) and 25061.141 (IT signal shape ON).

It can be tested with the w/f 24834.0 (default) and 24834.141 (IT signal shape ON)

This PR is not a backport.

@suchandradutta @mmusich @zdemirag

@cmsbuild cmsbuild changed the base branch from CMSSW_13_1_X to master April 4, 2023 16:13
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2023

@emiglior, CMSSW_13_1_X branch is closed for direct updates. cms-bot is going to move this PR to master branch.
In future, please use cmssw master branch to submit your changes.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2023

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41273/35035

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@emiglior emiglior force-pushed the add_customize_ITSignalShape_cmssw_13_1_X branch from 6901307 to 4f1410a Compare April 4, 2023 17:24
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41273/35037

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2023

A new Pull Request was created by @emiglior (Ernesto Migliore) for master.

It involves the following packages:

  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • SimTracker/SiPhase2Digitizer (upgrade, simulation)

@civanch, @bbilin, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET can you please review it and eventually sign? Thanks.
@fabiocos, @VourMa, @makortel, @threus, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @missirol, @kpedro88, @mmusich, @mtosi, @dgulhan, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Apr 4, 2023

test parameters:

  • relvals_opt= -w upgrade
  • workflows = 24834.0, 24834.141

@mmusich
Copy link
Contributor

mmusich commented Apr 4, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c86b79/31808/summary.html
COMMIT: 4f1410a
CMSSW: CMSSW_13_1_X_2023-04-04-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41273/31808/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • Reco comparison results: 9183 differences found in the comparisons
  • DQMHistoTests: Total files compared: 47
  • DQMHistoTests: Total histograms compared: 3365163
  • DQMHistoTests: Total failures: 5695
  • DQMHistoTests: Total nulls: 86
  • DQMHistoTests: Total successes: 3359360
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 46 files compared)
  • Checked 202 log files, 155 edm output root files, 47 DQM output files
  • TriggerResults: found differences in 2 / 45 workflows

@srimanob
Copy link
Contributor

srimanob commented Apr 8, 2023

@emiglior @mmusich
Any idea why we see failure in Run-3 DD4hep XML workflow comparison? If not, should the test be re-trigger, to make sure it clean? Thx.

@mmusich
Copy link
Contributor

mmusich commented Apr 9, 2023

Any idea why we see failure in Run-3 DD4hep XML workflow comparison?

As far as I know that wf has not a very good track record of stability

@srimanob
Copy link
Contributor

Thanks @mmusich
From your statement, it seems strange because the 11634 is the main workflow we use for Run-3. Using XML (.911) or DB (.0) should not give different result (i.e. instability of tracking). I don't see many failure of 11634.911 in PR test (under upgrade). Could you please point to any discussion or something happened before?

I don't want to hold the approval, just to make sure we understand the effect, specially on something we don't expect it to effect. Thanks for understanding.

@mmusich
Copy link
Contributor

mmusich commented Apr 10, 2023

@srimanob

Could you please point to any discussion or something happened before?

See please #41200 (or earlier #35109)

I don't want to hold the approval, just to make sure we understand the effect, specially on something we don't expect it to effect.

Up to you to decide, I am not pushing for this PR. Feel free to retrigger tests, these changes cannot possibly cause any change in Run3.

@makortel
Copy link
Contributor

@cmsbuild, please test

Let's try again. I don't think we have seen that massive differences in 11634.911 recently (or at least any mentions were not findable with GitHub search; #41200 is quite localized in terms of DQM histograms, and turned out to occur also in other workflows). It could be good to document this apparent non-reproducibility in a separate issue (we have seen some cases on non-reprocibility in SIM like in #34448)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c86b79/31895/summary.html
COMMIT: 96cf70e
CMSSW: CMSSW_13_1_X_2023-04-10-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41273/31895/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 10 lines from the logs
  • Reco comparison results: 12061 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3459609
  • DQMHistoTests: Total failures: 7724
  • DQMHistoTests: Total nulls: 83
  • DQMHistoTests: Total successes: 3451780
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 3 / 46 workflows

@srimanob
Copy link
Contributor

OK, the new test shows that the failure in Run-3 workflow is gone. Only Phase-2 workflows that show difference. I think this is expected.

@srimanob
Copy link
Contributor

+Upgrade

@emiglior
Copy link
Contributor Author

OK, the new test shows that the failure in Run-3 workflow is gone. Only Phase-2 workflows that show difference. I think this is expected.

I just want to confirm that the modifier introduced (.141) is supposed to run only in phase-2 w/f (it is checked against the presence of the phase-2 InnerTracker digitizers)

@sunilUIET
Copy link
Contributor

+pdmv

@cmsbuild
Copy link
Contributor

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants