cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.08k stars 4.3k forks source link

Cleaning GeometryExtended2026D49* from all config files #42945

Open srimanob opened 1 year ago

srimanob commented 1 year ago

D49 geometry is obsolete for long time, however there are still numbers of config files which keep it https://github.com/search?q=repo%3Acms-sw%2Fcmssw+GeometryExtended2026D49&type=code&p=1 I assume none of them are used.

This ticket is to keep track of cleaning the config files on Phase-2.

cmsbuild commented 1 year ago

A new Issue was created by @srimanob Phat Srimanobhas.

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

cms-bot commands are listed here

srimanob commented 1 year ago

assign upgrade

cmsbuild commented 1 year ago

New categories assigned: upgrade

@AdrianoDee,@srimanob you have been requested to review this Pull request/Issue and eventually sign? Thanks

mmusich commented 1 year ago

This is a recurrent issue. Developers that need or want to to create a test configuration for Phase 2, usually choose to use the default of that cycle, but then the default moves to something else and the test configuration remains stuck with the old geometry. Would it make sense to create a couple of files:

Configuration/Geometry/python/GeometryExtended2026Default_cff.py and Configuration/Geometry/python/GeometryExtended2026DefaultReco_cff.py

which contain just the import * of the contemporary default (e.g. now using D98)?. Then one can just use these and it will be automatically "forward ported" everywhere when the default changes.

srimanob commented 9 months ago

@mmusich @cms-sw/alca-l2 One thing just pops up to me is that even we make a default one, config still need to update the GT which include customization (i.e. T21), right? Is the GT defined in autoCond always point to the default geometry? https://github.com/cms-sw/cmssw/blob/master/Configuration/AlCa/python/autoCond.py#L93

Thx.

mmusich commented 9 months ago

Is the GT defined in autoCond always point to the default geometry?

normally yes, but it's not strictly enforced (at least to my knowledge).

srimanob commented 9 months ago

Thanks @mmusich

Question to @cms-sw/alca-l2 Is it possible to keep the default Phase-2 GT to match with default geometry baseline? I think this is the key to load default config and get everything properly. I understand that this is kind of manual book-keeping, but I don't expect we change drastically often.

perrotta commented 9 months ago

@srimanob the GT in autocond is supposed to point to the default geometry. If not, it should be notified and then updated. Yes, this is a manual procedure: please let AlCa know in case any update is needed for it.

mmusich commented 9 months ago

the GT in autocond is supposed to point to the default geometry. If not, it should be notified and then updated. Yes, this is a manual procedure: please let AlCa know in case any update is needed for it.

@perrotta, if that's the case, this situation is currently not verified for the master branch in which the default geometry is D98 (which should use auto:phase2_realistic_T25 as per

https://github.com/cms-sw/cmssw/blob/7924ce41e46a9c8f3b5fd7cc3ba2962a43ac0171/Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py#L2782

), while the autoCond key seems still to point to a T21 geometry:

https://github.com/cms-sw/cmssw/blob/7924ce41e46a9c8f3b5fd7cc3ba2962a43ac0171/Configuration/AlCa/python/autoCond.py#L93

please cross-check.

perrotta commented 9 months ago

Thank you @mmusich.

Therefore, if I understand correctly, the default geometry should be the one that corresponds to the T25 customization in Configuration/AlCa/python/autoCondPhase2.py, which is different from what currently in the DB (T21, as far as I can see).

The tracker config which corresponds to D98 should be T32 rather than T25. However, I imagine that they are similar enough that do not require separated tracker configurations (see Configuration/Geometry/README.md): correct?

In any case Configuration/AlCa/python/autoCondPhase2.py only customizes the Tracker geometry: is it enough to migrate just them , or should we also customize other subsystems in the GT, instead, to fully migrate to D98?

And, finally: @srimanob are you requesting to update the affected tags in the Phase2 default GT in order to synchronize it with the current default D98 update geometry? Or that "default" is expected to change often, so that the customizations are more practical, instead? In any case, this could be implemented when we'll derive the 140X GTs for Phase2.

mmusich commented 9 months ago

The tracker config which corresponds to D98 should be T32 rather than T25. However, I imagine that they are similar enough that do not require separated tracker configurations (see Configuration/Geometry/README.md): correct?

it is not only similar enough, it is identical, see

https://github.com/cms-sw/cmssw/blob/f5cbbf1ab5806ffc54ccaab6a77201f35d63fca7/Configuration/Geometry/README.md?plain=1#L77

In any case Configuration/AlCa/python/autoCondPhase2.py only customizes the Tracker geometry: is it enough to migrate just them

this is NOT the case, it customizes all the conditions that depend on the geometry.

or should we also customize other subsystems in the GT, instead, to fully migrate to D98?

as far as I am aware the Tracker is the only subsystem that supports different geometries (with geometry-dependent conditions) in the same release.

srimanob commented 9 months ago

Hi @perrotta

The default may change 2-3 times a year (~HGCal, Tracker, MTD at least once a year). I try to keep running the most recent geometry. As @mmusich states, only tracker that needs customization. So, maybe 1 (or 2) times a year that we update the tracker baseline, we will need your help to update the GT.

Regarding to move Phase-2 GT to T25, do you need something from Phase-2 tracker (or me), or any short talk at Alca meeting before the GT will be built. Thanks.

perrotta commented 9 months ago

Regarding to move Phase-2 GT to T25, do you need something from Phase-2 tracker (or me), or any short talk at Alca meeting before the GT will be built. Thanks.

It would be appropriate, indeed, in particular to decide the update strategies: thank you for proposing it, Phat. Would you be available already this Monday https://indico.cern.ch/event/1358549? Othewise next one could also be ok.

mmusich commented 9 months ago

It would be appropriate, indeed, in particular to decide the update strategies

Technically that's nothing terribly new (see here for some details). The main thing to manually enforce is just that the auto:phase2_realistic GT key is a "physical" copy of the "symbolic" auto:phase2_realistic_TXX corresponding to the geometry currently default in release (which was sort of implied when this arrangement was first stipulated, see also https://github.com/cms-sw/cmssw/pull/27517). This step is in any case always required for phase2 sample production as the production infrastructure is not technically capable of digesting symbolic GT keys.

EDIT : incidentally I think the tracker geometry for phase2 in converging (after some technical difficulties are overcome) to a final layout sometime this year, so this extra requirement would be fairly soon dropped.

perrotta commented 9 months ago

It would be appropriate, indeed, in particular to decide the update strategies

Technically that's nothing terribly new (see here for some details).

Thank you for the link. It is indeed almost five years old (I see that "2023" was the reference year for Phase2 in it...) and it is probably worth to quickly refresh the audience at the AlCa meeting.

The main thing to manually enforce is just that the auto:phase2_realistic GT key is a "physical" copy of the "symbolic" auto:phase2_realistic_TXX corresponding to the geometry currently default in release (which was sort of implied when this arrangement was first stipulated, see also #27517). This step is in any case always required for phase2 sample production as the production infrastructure is not technically capable of digesting symbolic GT keys.

This is indeed the main point: to decide when to manually update the "physical" GT key and the autoCond link: new campaigns, new releases, when the Upgrade contact tells AlCa that a new default geometry was agreed...

EDIT : incidentally I think the tracker geometry for phase2 in converging (after some technical difficulties are overcome) to a final layout sometime this year, so this extra requirement would be fairly soon dropped.

Great, some less manual work to adapt to it in sight, then!

srimanob commented 9 months ago

Hi @perrotta

I will be available on Monday for short update on phase 2 geometry and workflow. Thx.

perrotta commented 9 months ago

Hi @perrotta

I will be available on Monday for short update on phase 2 geometry and workflow. Thx.

Thank you Phat, added to the agenda: https://indico.cern.ch/event/1358549

mmusich commented 9 months ago

Thank you Phat, added to the agenda: https://indico.cern.ch/event/1358549

for those who didn't attend (since there are no minutes posted), would it be possible to summarize here the action items on this topic?

perrotta commented 9 months ago

Thank you Phat, added to the agenda: https://indico.cern.ch/event/1358549

for those who didn't attend (since there are no minutes posted), would it be possible to summarize here the action items on this topic?

perrotta commented 9 months ago

A new GT, 140X_mcRun4_realistic_v1, with the "default" T25 Phase2 tracker geometry was created

You can see the tag difference with respect to previous 133X_mcRun4_realistic_v1 in: https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/133X_mcRun4_realistic_v1/140X_mcRun4_realistic_v1

The new GT will be included in autocond in 140X as default GT for Phase2

saumyaphor4252 commented 9 months ago

The new GT will be included in autocond in 140X as default GT for Phase2

Here's the PR to include the above GT in autoCond: https://github.com/cms-sw/cmssw/pull/43763

srimanob commented 9 months ago

Thanks @cms-sw/alca-l2