Skip to content

FlatEvtVtxGenerator: do check on R/Phi limits only if UseCylindricalCoords is chosen and other fixes#50946

Merged
cmsbuild merged 4 commits into
cms-sw:masterfrom
cms-ngt-hlt:mm_fix_FlatEvtVtxGenerator
May 17, 2026
Merged

FlatEvtVtxGenerator: do check on R/Phi limits only if UseCylindricalCoords is chosen and other fixes#50946
cmsbuild merged 4 commits into
cms-sw:masterfrom
cms-ngt-hlt:mm_fix_FlatEvtVtxGenerator

Conversation

@mmusich
Copy link
Copy Markdown
Contributor

@mmusich mmusich commented May 15, 2026 •
edited
Loading

PR description:

Title says it all, in response of #50904 (comment).
I profit of this PR to introduce some other modernization.

PR validation:

runTheMatrix.py -l 7.0 -j 8 -t 4

now runs.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

N/A

@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 15, 2026

type bug-fix

@cmsbuild cmsbuild added this to the CMSSW_17_0_X milestone May 15, 2026
@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026 •
edited
Loading

cms-bot internal usage

@mmusich mmusich mentioned this pull request May 15, 2026
Merged
@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 15, 2026

test parameters:

  • workflows = 7.0, 7.1, 7.2, 7.21, 7.22, 7.23, 7.24, 7.3, 7.4, 7.5

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50946/49353

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

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

It involves the following packages:

  • IOMC/EventVertexGenerators (simulation)

@civanch, @cmsbuild, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks.
@fabiocos, @grzanka this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 15, 2026

@cmsbuild, please test

bfonta
bfonta reviewed May 15, 2026
View reviewed changes
Comment thread IOMC/EventVertexGenerators/src/FlatEvtVtxGenerator.cc Outdated
@mmusich
Modernize FlatEvtVtxGenerator configuration and initialization
8c8532e 
Refactor `FlatEvtVtxGenerator` to improve configuration validation, member initialization, and code clarity.
Replace the custom destructor with a defaulted one, reorganize member variables, and initialize common parameters directly in the constructor initializer list.
Update `fillDescriptions()` to use conditional parameter validation for Cartesian vs. polar (`FixedR`) configurations, and switch to `std::numbers::pi` for phi defaults.
Also improve logging formatting and clean up includes/comments for better readability and maintainability.
@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50946/49354

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

Pull request #50946 was updated. @civanch, @cmsbuild, @kpedro88, @mdhildreth can you please check and sign again.

@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 15, 2026

@cmsbuild, please test

@mmusich mmusich changed the title FlatEvtVtxGenerator: do check on R/Phi limits only if fixedR is chosen and other fixes FlatEvtVtxGenerator: do check on R/Phi limits only if UseCylindricalCoords is chosen and other fixes May 15, 2026
Dr15Jones
Dr15Jones reviewed May 15, 2026
View reviewed changes
Comment thread IOMC/EventVertexGenerators/interface/FlatEvtVtxGenerator.h Outdated
@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50946/49358

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

Pull request #50946 was updated. @civanch, @cmsbuild, @kpedro88, @mdhildreth can you please check and sign again.

@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 15, 2026

@cmsbuild, please test

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 15, 2026

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ebe08/53276/summary.html
COMMIT: a4030ea
CMSSW: CMSSW_17_0_X_2026-05-15-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/50946/53276/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 4 lines to the logs
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 61
  • DQMHistoTests: Total histograms compared: 4433348
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4433319
  • DQMHistoTests: Total skipped: 26
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 60 files compared)
  • Checked 270 log files, 224 edm output root files, 61 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Copy Markdown
Contributor

civanch commented May 16, 2026

+1

@cmsbuild
Copy link
Copy Markdown
Contributor

cmsbuild commented May 16, 2026

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

@mmusich
Copy link
Copy Markdown
Contributor Author

mmusich commented May 17, 2026 •
edited
Loading

urgent

  • a bunch of workflows keeps failing.

@mandrenguyen
Copy link
Copy Markdown
Contributor

mandrenguyen commented May 17, 2026

+1

@cmsbuild cmsbuild merged commit fb74581 into cms-sw:master May 17, 2026
11 checks passed
@mmusich mmusich deleted the mm_fix_FlatEvtVtxGenerator branch May 17, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@Dr15Jones Dr15Jones Dr15Jones left review comments

@bfonta bfonta bfonta left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug-fix code-checks-approved fully-signed orp-approved simulation-approved tests-approved urgent

Projects

None yet

Milestone

CMSSW_17_0_X

Development

Successfully merging this pull request may close these issues.

6 participants

@mmusich @cmsbuild @civanch @mandrenguyen @Dr15Jones @bfonta