-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Modules prohibiting concurrent lumis in RelVal workflows #25090
Comments
A new Issue was created by @gartung Patrick Gartung. @davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
See #25093 |
assign core |
New categories assigned: core @Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks |
Legacy modules in the ALCA workflows AlCaDiJetsProducer |
Legacy modules in DIGI2RAW workflows (can include DIGI, L1, HLT) BSCTrigger |
Legacy modules in RECO workflows (including DQM in many cases) BSCTrigger |
Legacy modules in HARVESTING workflows AlcaBeamMonitorClient |
@Dr15Jones these are correct now. 'grep -f legacy.modules.list' treated the module names as regex not exact match. |
@gartung may I kindly ask if you intend to change/update (or have changed) something in relvals outputs based on this issue? What is the purpose of it? |
assign simulation, reconstruction, dqm |
New categories assigned: dqm,reconstruction,simulation @mdhildreth,@jfernan2,@slava77,@andrius-k,@schneiml,@perrotta,@kmaeshima,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks |
@prebello I asked Patrick to gather the list of legacy modules. These modules are causing CPU efficiency problems, in particular they prohibit the framework from being able to processing multiple LuminosityBlocks concurrently. It would be best if the different L2 areas could 'poke' the responsible persons to get the various modules changed to be With that in mind, the core group will be spending effort in at least changing ones in the RECO workflow. |
I made a small modification to the framework so that if concurrent LuminosityBlocks are configured for a job, the framework will print out a list of modules which require synchronization on LuminosityBlock boundaries. With that in place I used workflow 1.0 to find modules: GEN+SIM:
DIGI:
RECO:
|
If I use workflow 137.8 and run step3 which does
Then the following module types cause LuminosityBlock synchronization
(Note, I dropped showing the module labels because of the huge number of different instances of the same type. If I kept the module label list the list would have been 789 entries long.) |
I used the Tracer service to determine what parts of workflow 1.0 need MEtoEDMConverter and ConditionDumperInEdm. Turns out no module consumes the output of MEtoEDMConverter while the PoolOutputModule does get the data from ConditionDumperInEdm. Even though no module wants data from MEtoEDMConverter, the framework is required to still call that module on Run and LuminosityBlock boundaries (since modules can have side-effects from those transitions). Given it is not doing anything useful, MEtoEDMConverter should be removed from RECO workflows. |
Slightly off topic, but sort of related: we should figure out which workflows use MEtoEDM/EDMtoME and for which reasons. The main consumer (to my knowledge) is Alca, but I am always surprised to hear how many WF load the module. Your observation that it gets loaded but not really used seems very plausible to me. |
The |
@schneiml using
I traced the inclusion of
where the cmsDriver created configuration file contains the line
|
Oh, that sounds like a fairly simple (and bad) situation. Thanks for digging out that old issue! And |
@gartung @Dr15Jones Perhaps a new list can be made, since the old list is 6 months old now. |
+1 |
+hlt As far I can see, there is no outstanding item for HLT here (if not, let me know, and I'll undo the signature). Thanks to @Dr15Jones, @Martin-Grunewald and @makortel for making the updates in #27961 #28175 #28445 #30987 (and maybe other PRs I missed). |
@gartung Could you close this issue? Thanks! |
@makortel following the discussion in #37071 concerning the
Maybe you could make a list of the "allowed" (preferred) solutions and start phasing out the ones that should be deprecated? (Sorry if this list already exists and I missed it!) Thanks for the help! |
According to documentation
that would be "no".
I believe that would be the preferred way. At least all uses of
If your suggestion is specifically about DQM modules, it would be up to @cms-sw/dqm-l2. Based on the documentation I'd avoid If your question was more general, only |
+l1 |
+1 |
This issue is fully signed and ready to be closed. |
I took the expanded python output from all of the gen-sim step of runTheMatrix jobs. From that I got a list of module names. Attached in the list of modules and their base class. There are two legacy modules, one EDAnalyzer and one EDFilter.
EDAnalyzer MEtoMEComparitor
EDFilter CosmicGenFilterHelix
gen-sim-modules.txt
The text was updated successfully, but these errors were encountered: