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

Restore CSC DQM monitor outside Muon POG sequence #46353

Open
fabiocos opened this issue Oct 11, 2024 · 15 comments
Open

Restore CSC DQM monitor outside Muon POG sequence #46353

fabiocos opened this issue Oct 11, 2024 · 15 comments

Comments

@fabiocos
Copy link
Contributor

IN CMSSW_14_0_18 the PR #46298, backport of #46293, has been integrated with the purpose of alleviating the memory issues faced by Tier0 in express harvesting workflows. It turns out that his updated has removed also CSC DQM monitoring, as reported in https://cms-talk.web.cern.ch/t/missing-folder-in-streamexpress-run2024i-express-v2-dqmio/52718/5 , since this sequenced has been not appropriately embedded in the Muon POG sequence, see https://github.com/cms-sw/cmssw/pull/46298/files#r1796815700 .

@rseidita is aware of the issue, and agreed to relocate the involuntarily removed sequence so as to restore the functionality. This issue is meant to record the problem and track its solution. As soon as the fix is available and backported to the CMSSW_14_0_X branch, a patch will be needed to restore the missing functionality in the Tier0.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 11, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @fabiocos.

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

cms-bot commands are listed here

@fabiocos
Copy link
Contributor Author

@ptcox FYI

1 similar comment
@fabiocos
Copy link
Contributor Author

@ptcox FYI

@makortel
Copy link
Contributor

assign dqm

@cmsbuild
Copy link
Contributor

New categories assigned: dqm

@antoniovagnerini,@nothingface0,@rvenditti,@syuvivida,@tjavaid you have been requested to review this Pull request/Issue and eventually sign? Thanks

@fabiocos
Copy link
Contributor Author

As an interim solution, pending a better organization of the muon DQM sequences to separate cscMonitor (and whatever else really needed) from the what can be really dropped without harm, I have opened #46364 , #46366 and #46365 to restore the previous situation as a clean basis, and allow the production of a quick patch release for Tier0 in 14_0_X @mandrenguyen @antoniovilela . This will unavoidably bring back the memory issues in T0 express harvesting, that were the reason for the change, but presumably to a smaller extent than adding directly the @muon sequence to the T0 configuration, that would activate additional folders and code filling them. The addition of the simple cscMonitor module to the T0 configuration is not possible, as it fails in the HARVESTING step configuration build (as observed independently while using cmsDriver.py within CMSSW).

@ptcox
Copy link
Contributor

ptcox commented Oct 17, 2024

Hi everyone. I tried to look into this unwanted coupling with Muon POG DQM and would like to suggest the following:

  1. In DQM/​CSCMonitorModule/​python/​csc_dqm_offlineclient_collisions_cff.py add cscMonitor just as is done for the TnPEfficiency sequence:
    cscOfflineCollisionsClients = cms.Sequence(dqmCSCOfflineClient+cscTnPEfficiencyClient) ->
    cscOfflineCollisionsClients = cms.Sequence(dqmCSCOfflineClient+cscMonitor+cscTnPEfficiencyClient)
  2. Remove the occurrences of cscMonitor in DQMOffline/Muon:
    DQMOffline/​Muon/​python/​muonCosmicMonitors_cff.py - 3 occurrences
    DQMOffline/Muon/python/muonMonitors_cff.py - 1 occurrence
    DQMOffline/Muon/test/muonMonitors_cff.py - 1 occurrence
    There is no case in which cscMonitor should occur alone, without the rest of CSC offline DQM, so once it's added to cscOfflineCollisionsClients all should remain as it should as far as CSC DQM is concerned.
    I think this splits cscMonitor out of Muon POG DQM.
    Does anyone see anything wrong with this?

@rseidita
Copy link
Contributor

Hi Tim, thanks a look for taking the time to look into this. It looks good to me; I will implement your suggestion, test it, and let you know

@ptcox
Copy link
Contributor

ptcox commented Oct 17, 2024

Thanks for looking it. I'm now concerned I don't understand how things really work though - I am getting confused between cscTnPEfficiencyClient and cscTnPEfficiencyMonitor when trying to make the analogy with the TnPEfficiency.

@ptcox
Copy link
Contributor

ptcox commented Oct 17, 2024

Yes, I am wrong: one needs a DQMEDHarvester to add to cscOfflineCollisionsClients. I don't know how CSCOfflineMonitor histograms get harvested.

@rseidita
Copy link
Contributor

rseidita commented Oct 17, 2024

Something that seems to work is simply adding the cscMonitor to the DQMOfflineMuonDPGExpress sequence here. I checked that, with the POG sequence commented out (the thing that triggered this whole discussion), I do indeed get back the CSC/CSCOfflineMonitor folder when running

cmsDriver.py step2 --conditions 140X_dataRun3_Express_v3 --data --datatier RECO,DQMIO --era Run3_2024 --eventcontent RECO,DQM --filein /store/express/Run2024I/ExpressPhysics/FEVT/Express-v2/000/386/814/00000/fa7e3b68-1bb0-4173-a047-271497361731.root --fileout file:step2_modified.root --nStreams 2 --nThreads 8 --no_exec --number 10 --process RECODQM --python_filename step2_modified_cfg.py --scenario pp --step RAW2DIGI,L1Reco,RECO,DQM:@standardDQMExpress

and

cmsDriver.py step3 --conditions 140X_dataRun3_Express_v3 --data --era Run3_2024 --filein file:step2_modified_inDQM.root --fileout file:step3_modified.root --filetype DQM --nStreams 2 --no_exec --number 10 --python_filename step_3_modified_cfg.py --scenario pp --step HARVESTING:@standardDQMExpress

Though I didn't touch the harvesting part at all, so I have to fully convince myself that this is enough

@ptcox
Copy link
Contributor

ptcox commented Oct 17, 2024

Yes, thanks for trying. I think it may be that cscMonitor should be added to cscSources, which would also replace your addition above.
But then somewhere those histograms must get harvested. I still don't see where. Something that does that harvesting then would need to be added to cscOfflineCollisionsClients. I wonder if the harvesting is somehow implicit in some existing DQMEDHarvester which already handles things. T

@mmusich
Copy link
Contributor

mmusich commented Oct 17, 2024

I wonder if the harvesting is somehow implicit in some existing DQMEDHarvester which already handles things.

that's correct. Normally any HARVESTING:@whatever does the EDM to ME conversion by default (which is the minimum to get the plots harvested in the output root file). You need to specify a specific sequence only if you need to post-process the output of the previous step in a more refined way (e.g. compute efficiencies, fake rates, fitting, etc.).

@ptcox
Copy link
Contributor

ptcox commented Oct 17, 2024

Thanks for that @mmusich. CSCOfflineMonotor doesn't actually calculate anything at the end (although it should - but nobody ever got around to implementing anything - there was a plan to calculate efficiencies I think.)
So maybe the solution really is just to add cscMonitor to cscSources. That is, in
DQM/CSCMonitorModule/python/csc_dqm_sourceclient_offline_cff.py where there is currently
cscSources = cms.Sequence(dqmCSCClient + cscTnPEfficiencyMonitor)

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

6 participants