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

Config files needed to integrate PPS simhits into the cms simulation #27990

Merged
merged 2 commits into from Sep 29, 2019

Conversation

mundim
Copy link
Contributor

@mundim mundim commented Sep 13, 2019

PR description:

New Eras file for PPS and change in the Configuration/StandardSequences and g4SimHits in order to integrate PPS into the CMS simulation (only sim hits).

PR validation:

scram b code-format
scram b runtests
and runTheMatrix have been executed, with the output:
22 18 17 16 9 3 0 0 0 tests passed, 12 3 0 0 0 0 0 0 0 failed

if this PR is a backport please specify the original PR:

Before submitting your pull requests, make sure you followed this checklist:

@cmsbuild cmsbuild added this to the CMSSW_11_0_X milestone Sep 13, 2019
@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27990/11885

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mundim for master.

It involves the following packages:

Configuration/Eras
Configuration/StandardSequences
SimG4Core/Application

@civanch, @kpedro88, @cmsbuild, @franzoni, @mdhildreth, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@civanch
Copy link
Contributor

civanch commented Sep 13, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 13, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2515/console Started: 2019/09/13 20:03

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27990/11890

  • This PR adds an extra 24KB to repository

  • Found files with invalid states:

    • Configuration/Eras/python/Modifier_pps_2018_cff.py:
    • Configuration/Eras/python/Modifier_pps_2016_cff.py:
    • Configuration/Eras/python/Modifier_pps_2017_cff.py:

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@civanch
Copy link
Contributor

civanch commented Sep 16, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 16, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2520/console Started: 2019/09/16 09:23

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2957336
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2956994
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 145 log files, 15 edm output root files, 34 DQM output files

@civanch
Copy link
Contributor

civanch commented Sep 18, 2019

+1

@fabiocos
Copy link
Contributor

@civanch @mundim sorry what is the purpose of this PR? In which workflow am I supposed to see effects, i.e. changes in event content? Are all the needed collections already defined?
I would like that we really move forward with this simulation, could we have a plan to progressively see updates in a well defined test workflow?

@mundim
Copy link
Contributor Author

mundim commented Sep 23, 2019

@fabiocos, sorry to have not included explicitly the originating PR where this one was requested (27608). This PR includes config files to create

CTPPSPixelHits
CTPPSTimingHits
TotemHitsRP

and yes, all the collections (for sim hit) are defined and merged.

The test file in SimPPS/RPDigiProducer/test can be used to test it and a change in the SimPPS/PPSPixelDigiProducer/test will be issued soon, however, these are to test the Digi (if the simhit is not present they will fail).

We do have a better config test, which was planed for the next PR with all the remaining config files. It is possible to issue a PR with only SimPPS/Configuration to so so, but they require SimPPS/PPSPixelDigiProducer and SimPPS/RPDigiProducer already merged unless I include only the config to test the step 1 (SIM).

Unless you advise otherwise, my plan is to issue a single PR (after the current one) with all remaining config files. Let me know what you all prefer.

@fabiocos
Copy link
Contributor

@mundim thank you for the clarification, but so far this PR and #27608 seem of limited use in getting at least a partly working workflow. Your standalone test may be ok for private studies, but are you planning to have a complete production geometry in a normal test workflow?
@civanch this PR activates the SDs, but is PPS fully part of the navigation (G4regions, production cuts, etc..)?
In any case the test code mentioned does not produce any digi output, so that part cannot be tested

@fabiocos
Copy link
Contributor

+operations

the update of Era configuration looks technically correct

@fabiocos
Copy link
Contributor

+1

@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 be automatically merged.

@fabiocos
Copy link
Contributor

@mundim @civanch to help moving forward, still I would like an answer to my questions

@mundim
Copy link
Contributor Author

mundim commented Sep 30, 2019

As I said in the PR 27608, I'm going to submit a new PR with the config files that will integrate all pieces together. Since it was requested to submit one PR, wait until it gets merged to submit the next one, there was no way (at least it was my undestanding) to have the full picture until all PRs have been submitted.

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

6 participants