PR for HLT online DQM monitoring code for Proton-Lead run in 80X (upd…

[varuns23]

…ated #16355

@mtosi @silviodonato @diguida @vanbesien @dmitrijus

PR made using CMSSW_8_0_X_2016-11-02-1100

[cmsbuild]

A new Pull Request was created by @varuns23 (varun sharma) for CMSSW_8_0_X.

It involves the following packages:

DQM/HLTEvF
DQM/Integration

@perrotta, @cmsbuild, @silviodonato, @dmitrijus, @Martin-Grunewald, @fwyzard, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@threus, @batinkov this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

[diguida]

@varuns23 thanks for the follow-up changes. Can you please add this PR to the DQM Twiki for 2016 pPb?

[varuns23]

Added

[Martin-Grunewald]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16153/console

[Martin-Grunewald]

I see you have renamed HeavyIons to ProtonLead for some files and changed them a bit.
Shoudn't one keep the original HeavyIons files around as we will have PbPb runs in the future?

[varuns23]

Yes, the code structure need some modification. But as there is limited time for Proton-Lead run, we went ahead with the present setup and renamed HeavyIon to ProtonLead as suggested by Salvatore. Moreover PbPb run is not going to happen until late 2018. So removed it to avoid any confusion.

[diguida]

@Martin-Grunewald Besides what @varuns23 just pointed out, in the current implementation of the "Heavy Ion" module, the monitored paths are specific for "Proton Lead", which looks even more confusing, IMHO.

[davidlange6]

is there an actual change to the plugin besides a cool new name?

[varuns23]

Yes, there are some changes besides name change. The important one is in the hlt_dqm_sourceclient-live_cfg.py config file, where it now loads the protonlead module. Earlier it was set to pick module depending on the run key and we are having the same run key as pp for PA so it was not using the correct module.

[diguida]

@davidlange6 As pointed out by @varuns23 the most important change is for DQM online operations. For the rest, apart from the plugin renaming, there are two minor fixes in the plugin itself:

• load event collections whose names are provided in the configuration;
• remove the token for an event collection not used afterwards.

[davidlange6]

so keep the old name of the plugin, make the trivial functionality change (the unused token) and just make a new cfi with the cool new name. Then by doing less work you preserve the PbPb functionality and get the new pPb functionality

[varuns23]

@davidlange6 I did not get your point. You want me to revert to HLTObjectMonitorHeavyIon.cc and rename which cfi?

[davidlange6]

On Nov 2, 2016, at 4:53 PM, varun sharma notifications@github.com wrote:

@davidlange6 I did not get your point. You want me to revert to HLTObjectMonitorHeavyIon.cc and rename which chi?

Right - rename nothing, make a new config with your pA specific name and changes

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[diguida]

@davidlange6 what you propose is the ideal fix. Even better, there should be no need to define a new plugin for HI or pA either, but just to define a new instance of the plugin with the new name and the correct configuration. BTW, it has been never created a plugin for PbPb, in the version of the "Heavy Ion" module in CMSSW_8_1_X_2016-11-02-1100, the monitored paths are specific for "Proton Lead", which looks even more confusing, IMHO.

The joint modifications of this and #16355 PRs allow to monitor some specific pPb paths for the pPb data taking period only.

On the longer term, DQM and STEAM have discussed some ideas to completely restructure the code, in order to make the C++ module deal with the paths defined in the HLT menu of the analysed run; as it is now, the C++plugin depends on the python configuration, which is a design flaw.

[davidlange6]

ok so its just a big mess

On Nov 2, 2016, at 4:58 PM, Salvatore Di Guida notifications@github.com wrote:

@davidlange6 what you propose is the ideal fix. Even better, there should be no need to define a new plugin for HI or pA either, but just to define a new instance of the plugin with the new name and the correct configuration. BTW, it has been never created a plugin for PbPb, in the version of the "Heavy Ion" module in CMSSW_8_1_X_2016-11-02-1100, the monitored paths are specific for "Proton Lead", which looks even more confusing, IMHO.

The joint modifications of this and #16355 PRs allow to monitor some specific pPb paths for the pPb data taking period only.

On the longer term, DQM and STEAM have discussed some ideas to completely restructure the code, in order to make the C++ module deal with the paths defined in the HLT menu of the analysed run; as it is now, the C++plugin depends on the python configuration, which is a design flaw.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[diguida]

@davidlange6 practically, yes :-(

[varuns23]

@davidlange6 Yes, its a bit complicated. We have already taken suggestion from @diguida and working for a better structured setup. But I think for now it is important to monitor the present Proton-Lead run and the current setup should be able to do it.

[cmsbuild]

+1
Tested at: 17852e4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16430/16153/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-16430/16153/summary.html

[dmitrijus]

+1

[varuns23]

Any update from hlt or orop side?

@mtosi @silviodonato @diguida @vanbesien

[Martin-Grunewald]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

[davidlange6]

+1