Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phase2TrackerDigitizer: Activating Bias Rail Inefficiency #40069

Merged

Conversation

suchandradutta
Copy link
Contributor

@suchandradutta suchandradutta commented Nov 15, 2022

PR description:

Activating the bias rails inefficiency for PS-p modules by default as it is the intrinsic to the sensor itself. SimHits are discarded (not considered for Digitization) when the trajectory is entirely inside the bias rail region. At the moment we consider also those SimHits for which the trajectory is partly outside (optimistic scenario with 'AND' condition for Entry/Exit points of SimHits) the bias rail region.

Note : the code was already integrated before we are now switching it on through a configuration parameter.

We expect to find less digis in PSP modules due to this inefficiency

At the moment we are estimating the effect of this inefficiency in L1 tracking and it is not fully optimized. We might move to a more pessimistic scenario when we shall not consider the SimHits where the the trajectory is event partly inside the bias rail region ( 'OR' condition for Entry/Exit points of SimHits)

PR validation and some performance updates

Note : all the above validation plots are presented with the pessimistic 'OR' option

@emiglior

… it is the intrinsic to the sensor itself. SimHits are discarded when the trajectory is entirely inside the bias rail region. At the moment we consider the SimHits for which the trajectory is partly outside the bias rail region
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40069/33053

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @suchandradutta (Suchandra Dutta) for master.

It involves the following packages:

  • SimTracker/SiPhase2Digitizer (upgrade, simulation)

@cmsbuild, @AdrianoDee, @srimanob, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@beaucero, @VourMa, @mtosi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @missirol, @trtomei, @mmusich, @threus, @dgulhan 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 Nov 16, 2022

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fc2b6b/29040/summary.html
COMMIT: 5dde5a0
CMSSW: CMSSW_12_6_X_2022-11-15-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40069/29040/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11945 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3417074
  • DQMHistoTests: Total failures: 6648
  • DQMHistoTests: Total nulls: 84
  • DQMHistoTests: Total successes: 3410320
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 2 / 46 workflows

@mmusich
Copy link
Contributor

mmusich commented Nov 16, 2022

please test

  • spurious failure

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fc2b6b/29061/summary.html
COMMIT: 5dde5a0
CMSSW: CMSSW_12_6_X_2022-11-16-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40069/29061/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11949 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3417074
  • DQMHistoTests: Total failures: 6654
  • DQMHistoTests: Total nulls: 84
  • DQMHistoTests: Total successes: 3410314
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 2 / 46 workflows

@civanch
Copy link
Contributor

civanch commented Nov 17, 2022

@suchandradutta , I agree with proposed implementation but am not sure if it is a correct approach to add any filter if the geometry the new tracker is not fully finalized. Usually such type of filter or mask are added to very final variant of simulation.

@emiglior
Copy link
Contributor

Hi @civanch, as mentioned in the PR description, this inefficiency is kind of unavoidable.
For the moment, we would prefer to keep the activation of the mask as an option to make the comparison with past results easier and to allow the development of mitigation strategies.

@civanch
Copy link
Contributor

civanch commented Nov 17, 2022

+1

@srimanob
Copy link
Contributor

+Upgrade

Maybe OR and AND should be control via configuration in case of the need of comparison in future.

Changes are seen in all Phase-2 workflows as expected since the PR effects only Phase-2. Note to @cms-sw/pdmv-l2 on the validation when the release with this PR comes out.

@mmusich
Copy link
Contributor

mmusich commented Nov 18, 2022

please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40069/33092

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #40069 was updated. @AdrianoDee, @srimanob, @civanch, @mdhildreth can you please check and sign again.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fc2b6b/29122/summary.html
COMMIT: 024533d
CMSSW: CMSSW_12_6_X_2022-11-18-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40069/29122/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test-das-selected-lumis had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11970 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3417167
  • DQMHistoTests: Total failures: 6650
  • DQMHistoTests: Total nulls: 84
  • DQMHistoTests: Total successes: 3410411
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 2 / 46 workflows

@mmusich
Copy link
Contributor

mmusich commented Nov 18, 2022

please test

  • failure is spurious

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fc2b6b/29130/summary.html
COMMIT: 024533d
CMSSW: CMSSW_12_6_X_2022-11-18-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40069/29130/install.sh to create a dev area with all the needed externals and cmssw changes.

  • DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11974 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3417167
  • DQMHistoTests: Total failures: 6653
  • DQMHistoTests: Total nulls: 84
  • DQMHistoTests: Total successes: 3410408
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 2 / 46 workflows

@civanch
Copy link
Contributor

civanch commented Nov 19, 2022

+1

@srimanob
Copy link
Contributor

+Upgrade

Re-sign

@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)

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit d0652c4 into cms-sw:master Nov 21, 2022
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.

None yet

9 participants