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

Redefine the deafult behavior of the --beamspot option in cmsDriver #47182

Open
francescobrivio opened this issue Jan 24, 2025 · 13 comments
Open

Comments

@francescobrivio
Copy link
Contributor

Issue Description

As reported to me by @missirol and @malbouis, it happened recently that in a MC campaign (Winter25, see this GEN-SIM driver for example) the --beamspot option was not specified in the cmsDriver.

After digging a little this is what I understand the current behavior in such cases is:

  • In the configBuilder a VtxSmearedDefaultKey is set:
    if not self._options.beamspot:
    self._options.beamspot=VtxSmearedDefaultKey
  • The default value is read from
    VtxSmearedDefaultKey='Realistic50ns13TeVCollision'
  • Which eventually instructs the framework to use a VertedSmearing scenario from 2015:
    # From 2015B 3.8T data
    # Centroid absolute positions extracted from fill 4008:
    # X = 0.07798 cm
    # Y = 0.09714 cm
    # Z = -1.610 cm
    #
    # BPIX absolute position extracted from PCL-like alignment run after magnet ramp-up:
    # X = -0.026837 cm
    # Y = -0.0715252 cm
    # Z = -0.511453 cm
    Realistic50ns13TeVCollisionVtxSmearingParameters = cms.PSet(
    Phi = cms.double(0.0),
    BetaStar = cms.double(65.0),
    Emittance = cms.double(5.411e-08),
    Alpha = cms.double(0.0),
    SigmaZ = cms.double(5.3),
    TimeOffset = cms.double(0.0),
    X0 = cms.double(0.10482),
    Y0 = cms.double(0.16867),
    Z0 = cms.double(-1.0985)
    )

    Thus effectively invalidating the MC campaign...

Possible Solutions

In order two avoid this happening again in the future, we (discussing with @mmusich) believe there are two possible options:

  1. Change the VtxSmearedDefaultKey value to be DBrealistic, which instructs the framework to read the VtxSmearing scenario from the GT (as introduced in Read VertexSmearing from GT in Run-1/2/3 MC #43242).
  2. Modifying the configBuilder itself to raise and exception in case the --beamspot parameter has not been set in the cmsDriver.

Both options are equally valid IMHO, so I'd like the opinion of @cms-sw/core-l2 @cms-sw/pdmv-l2.

FYI @cms-sw/ppd-l2 @missirol @mtosi

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 24, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @francescobrivio.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@francescobrivio
Copy link
Contributor Author

assign core,pdmv

@cmsbuild
Copy link
Contributor

New categories assigned: core,pdmv

@AdrianoDee,@DickyChant,@Dr15Jones,@makortel,@miquork,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

unassign core

Core does not maintain cmsDriver

@mmusich
Copy link
Contributor

mmusich commented Jan 24, 2025

assign operations

@cmsbuild
Copy link
Contributor

New categories assigned: operations

@antoniovilela,@davidlange6,@fabiocos,@mandrenguyen,@rappoccio you have been requested to review this Pull request/Issue and eventually sign? Thanks

@francescobrivio
Copy link
Contributor Author

unassign core

Core does not maintain cmsDriver

Ups sorry Matti! I thought configBuilder was important enough to be under core's umbrella 😄

@makortel
Copy link
Contributor

  • Change the VtxSmearedDefaultKey value to be DBrealistic, which instructs the framework to read the VtxSmearing scenario from the GT (as introduced in Read VertexSmearing from GT in Run-1/2/3 MC #43242).
  • Modifying the configBuilder itself to raise and exception in case the --beamspot parameter has not been set in the cmsDriver.

How often there are cases where --beamspot has to be something else than DBrealistic?

@AdrianoDee
Copy link
Contributor

AdrianoDee commented Jan 24, 2025

How often there are cases where --beamspot has to be something else than DBrealistic?

I think whenever (@francescobrivio please correct me in case):

a. one uses a GT not including the BS (that I think it is now limited to cases when one want's to re-run on some older samples?);
b. one wants to use DBdesign .

Given this I would vote for:

  1. Modifying the configBuilder itself to raise and exception in case the --beamspot parameter has not been set in the cmsDriver.

to catch any problem already at config level rather than at runtime (e.g. when in case a. one forgets to specify --beamspot XX).

@makortel
Copy link
Contributor

How often there are cases where --beamspot has to be something else than DBrealistic?

I think whenever (@francescobrivio please correct me in case):

a. one uses a GT not including the BS (that I think it is now limited to cases when one want's to re-run on some older samples); b. one wants to use DBdesign .

Given this I would vote for:

  1. Modifying the configBuilder itself to raise and exception in case the --beamspot parameter has not been set in the cmsDriver.

to catch any problem already at config level rather than at runtime (e.g. when in case a. one forgets to specify --beamspot).

I agree. (core generally favors reporting errors early and loudly)

@mmusich
Copy link
Contributor

mmusich commented Jan 24, 2025

to catch any problem already at config level rather than at runtime (e.g. when in case a. one forgets to specify

For completeness, I just want to mention that if the GlobalTag is self-consistent among the GEN and RECO beanspot records (@cms-sw/alca-l2 should guarantee that), there's no need to throw at configuration or runtime (and if it's not self-consistent the campaign would be wrong even if you pass the DBRealistic option by hand).

b. one wants to use DBdesign .

I think that if you don't specify it when you would need to, it leads to crashes at runtime (see https://gitlab.cern.ch/cms-ppd/cms-ppd-matters/-/issues/52#note_8968304 ). Perhaps could crash more gracefully ;)

@mmusich
Copy link
Contributor

mmusich commented Jan 27, 2025

I think that if you don't specify it when you would need to, it leads to crashes at runtime (see https://gitlab.cern.ch/cms-ppd/cms-ppd-matters/-/issues/52#note_8968304 ). Perhaps could crash more gracefully ;)

I followed up at #47189.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants