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

Hepmc3tog4, addind the codes that provide a possibility to transfer HepMC3Product from the GEN step to the SIM step #46797

Merged
merged 16 commits into from
Nov 29, 2024

Conversation

mkirsano
Copy link
Contributor

This is a continuation of the work described in the issue #36662
The added code allows to transfer the HepMC3 event record to Sim and load particles in Geant4. It is NOT USED if HepMC record is produced in the Genarator part.
Two test configurations are added in Pythia8Interface/test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 25, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • GeneratorInterface/Core (generators)
  • GeneratorInterface/Pythia8Interface (generators)

@bbilin, @cmsbuild, @lviliani, @menglu21, @mkirsano can you please review it and eventually sign? Thanks.
@alberto-sanchez this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46797/42799

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

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46797/42800

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #46797 was updated. @bbilin, @civanch, @cmsbuild, @kpedro88, @lviliani, @mdhildreth, @menglu21, @mkirsano, @tvami can you please check and sign again.

@mkirsano
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-76e16b/43144/summary.html
COMMIT: 93d1cf1
CMSSW: CMSSW_15_0_X_2024-11-28-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46797/43144/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: 44 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3484682
  • DQMHistoTests: Total failures: 1888
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3482774
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
  • Checked 202 log files, 172 edm output root files, 46 DQM output files
  • TriggerResults: found differences in 1 / 44 workflows

@civanch
Copy link
Contributor

civanch commented Nov 29, 2024

+1

@mkirsano
Copy link
Contributor Author

+1

}
}
const HepMC3::GenParticle *ppointer = pitr.get();
if (fLumiFilter && !fLumiFilter->isGoodForLumiMonitor(ppointer)) { // MK: this function is always true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment on the purpose of this? isGoodForLumiMonitor is indeed always true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from the class Generator, written by somebody else for HepMC, time ago. May be there is a possibility to provide another, non-trivial function for special cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but it's ok to do that in another PR if you prefer

@tvami
Copy link
Contributor

tvami commented Nov 29, 2024

And I'd also suggest to expand the PR title to be a bit more descriptive, thanks!

@mkirsano mkirsano changed the title Hepmc3tog4 Hepmc3tog4, addind the codes that provide a possibility to transfer HepMC3Product from the GEN step to the SIM step Nov 29, 2024
@tvami
Copy link
Contributor

tvami commented Nov 29, 2024

+analysis

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

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 94bc4bd into cms-sw:master Nov 29, 2024
11 checks passed
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.

5 participants