Skip to content

Conversation

@hahnjo
Copy link
Contributor

@hahnjo hahnjo commented Oct 14, 2022

PR description:

The new EMH physics list replaces the EM processes for electrons, positrons, and gammas with G4HepEm. Note that gamma-lepto-nuclear interactions are NOT implemented yet, and disabled for this physics list (by removing G4EmExtraPhysics).

NOTE: depends on cms-sw/cmsdist#8128

PR validation:

Running with process.g4SimHits.Physics.type = 'SimG4Core/Physics/FTFP_BERT_EMH' works, validation of this experimental physics list will follow.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39736/32579

ERROR: Build errors found during clang-tidy run.

SimG4Core/PhysicsLists/src/CMSEmStandardPhysicsEMH.cc:25:10: error: 'G4HepEmProcess.hh' file not found [clang-diagnostic-error]
#include "G4HepEmProcess.hh"
         ^
Suppressed 1147 warnings (1146 in non-user code, 1 with check filters).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@smuzaffar
Copy link
Contributor

code-checks with cms.week0.PR_b82b891d/52.0-5cce5a899ed266ff8656f99553f33e4a

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39736/32591

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @hahnjo (Jonas Hahnfeld) for master.

It involves the following packages:

  • BigProducts/Simulation (core, simulation)
  • SimG4Core/PhysicsLists (simulation)

@smuzaffar, @civanch, @Dr15Jones, @makortel, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@makortel, @bsunanda, @rovere, @fabiocos, @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

@smuzaffar
Copy link
Contributor

test parameters:

@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ef250/28279/summary.html
COMMIT: 45e5ba4
CMSSW: CMSSW_12_6_X_2022-10-14-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39736/28279/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-8ef250/2500.601_mc126X+TTBarMINIAOD12.6+NANO_mc12.6+HRV_NANO_mc

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • Reco comparison had 6 failed jobs
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3391158
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3391133
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 201 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Oct 16, 2022

@hahnjo , does G4HepEM support models per region? In EMM we have different configurations of multiple scattering for HCAL and for the rest.

@hahnjo
Copy link
Contributor Author

hahnjo commented Oct 16, 2022

@civanch no, different MSC configurations per region are not (yet) supported. This PR is only for the initial integration and making sure that everything works as expected

@civanch
Copy link
Contributor

civanch commented Oct 16, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 9, 2022

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ef250/28905/summary.html
COMMIT: 9edb037
CMSSW: CMSSW_12_6_X_2022-11-08-2000/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39736/28905/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ef250/28905/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ef250/28905/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test PVall had ERRORS
---> test DMRall had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 443 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3416402
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3416374
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 4.14 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 11634.0,... ): 0.556 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 13234.0,... ): 0.268 KiB Physics/NanoAODDQM
  • Checked 206 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@hahnjo
Copy link
Contributor Author

hahnjo commented Nov 10, 2022

@civanch @smuzaffar I don't see how the failure can be related to my changes, the new physics list isn't even active by default...

@civanch
Copy link
Contributor

civanch commented Nov 11, 2022

@smuzaffar , we discussed with @hahnjo and he propose to test this PR without extra external build. Does this make sense?

@civanch
Copy link
Contributor

civanch commented Nov 11, 2022

please test

@civanch
Copy link
Contributor

civanch commented Nov 11, 2022

I restart testing, because in previous tests the problem was in access to file in calibration unit tests. This should not depend on this PR.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ef250/28975/summary.html
COMMIT: 9edb037
CMSSW: CMSSW_12_6_X_2022-11-11-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39736/28975/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3416447
  • DQMHistoTests: Total failures: 9
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3416416
  • 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: no differences found

@civanch
Copy link
Contributor

civanch commented Nov 11, 2022

+1

@civanch
Copy link
Contributor

civanch commented Nov 14, 2022

@smuzaffar , it seems that PR is OK.

@civanch
Copy link
Contributor

civanch commented Nov 17, 2022

@makortel , may be you can approve this PR? We converged since several days. It should not affect any standard WF in this form.

@makortel
Copy link
Contributor

I was hoping @smuzaffar would sign

@smuzaffar
Copy link
Contributor

+core
sorry I thought I already have signed it

@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

@cmsbuild cmsbuild merged commit 2a05927 into cms-sw:master Nov 22, 2022
@hahnjo hahnjo deleted the g4hepem branch November 23, 2022 07:43
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.

6 participants