Phase 1 Pixel DQM (number 4, the story continues)

[schneiml]

See also #14230 . Everything important is there.

Everything is rebased to -pre5 now.

@dmitrijus For you this PR adds more DQM plugins and therefore more histograms. Apart from that not much has changed. (I did fix some of the whitespace issues.)

@slava77 @davidlange6 TrackerTopology now has a new interface as discussed in #14230 .
I only implemented it for Pixel, but it would obviously work as well for the rest of the Tracker. But I am a bit reluctant to add that now, since a typo there would be likely, almost impossible to spot and could bite really badly if someone started to use it.

There are now 8 new packages. One with shared code, one with shared config and 6 plugins.

[cmsbuild]

Pull request #14586 was updated. @cvuosalo, @dmitrijus, @cmsbuild, @deguio, @slava77, @vanbesien, @davidlange6 can you please check and sign again.

[slava77]

@cmsbuild please test

[cmsbuild]

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

[cmsbuild]

+1
Tested at: dcf132f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14586/13113/summary.html

[cmsbuild]

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

[slava77]

+1

for #14586 dcf132f

• code changes in reco are only in TrackerTopology.h/cc: less exposing and more generic interface is introduced to figure out mapping of detector elements
• jenkins tests pass and comparisons with baseline show no differences

[boudoul]

@dmitrijushttps://github.com/dmitrijus @vanbesienhttps://github.com/vanbesien
please consider this PR
Thanks
Gaelle

Le 20 mai 2016 à 15:15, Slava Krutelyov <notifications@github.commailto:notifications@github.com> a écrit :

@dmitrijushttps://github.com/dmitrijus @vanbesienhttps://github.com/vanbesien
once you are happy with the package names, please make a PR to cmssw/cms-bot with edits to
https://github.com/cms-sw/cms-bot/blob/master/categories.py#L889
this can be edited on the web
once merged, the new-package-pending issue with this PR will be cleared

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//pull/14586#issuecomment-220602259

[dmitrijus]

+1

[dmitrijus]

cms-sw/cms-bot#681

[cmsbuild]

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

[slava77]

This is just a note/warning at this point: this PR broke data processing configuration

python $CMSSW_RELEASE_BASE/src/Configuration/DataProcessing/test/RunPromptReco.py \
--scenario ppEra_Run2_2016 --reco --aod --miniaod --dqmio --global-tag 80X_dataRun2_Prompt_v8 \
 --lfn=/store/data/Run2016B/JetHT/RAW/v2/000/274/157/00000/1C660F1D-FE24-E611-AACE-02163E0123A1.root  \
--alcareco SiStripCalZeroBias+SiStripCalMinBias+TkAlMinBias  --dqmSeq @common \
--PhysicsSkim=LogError+LogErrorMonitor

the generated RunPromptRecoCfg.py works in plain pre5
and it fails in pre5+this PR with the following error

An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named RunPromptRecoCfg.py
Exception Message:
python encountered the error: <type 'exceptions.AttributeError'>
'module' object has no attribute 'Specification'
----- End Fatal Exception -------------------------------------------------

we probably still have time to fix in 81X.
I suppose there is no intent to backport to 80X.

[schneiml]

From a quick look I suspect a bug in the script that generates this config (I have no idea how it works and have not yet looked into it).

It is mislead by the fact the the Phase1 DQM config contains subclasses of PSet, and seems to wrongly assume that cms.<classname>(<values>) can be used to construct one. This is wrong in many ways, since the class Specification in question lives in the SiPixelPhase1Common package and does not allow construction like this.

The correct way would be to construct this object as cms.PSet, since in the end it is just that and will also be passed to C++ as a PSet. Maybe I have to overload __repr__ or sth, as said, I have not looked at the code yet.

Anyways, I think the bug is not on my side but in the script that ignores Python semantics and will also break on the (few) other subclasses of PSet.

Update: the misbehaviour seems to be in _ModuleSequenceType.dumpPython() (FWCore/ParameterSet/python/SequenceTypes.py:229). It could be fixed there by looking for the closest parent in the cms package or by a local override (which seems much easier, I hope this is actually possible).

[slava77]

@Dr15Jones is on vacation, unfortunately,
we may need to come back to this later.
It sounds to me that some changes made in this PR are outside of the design of the cms PSet

[schneiml]

I am in the moment struggling to get anything to run at all, but from what I saw before, the fix in #14760 should be fine. I got a Python error because a sequence contained ...++.... Afair it was L1T stuff, so I think it is unrelated.

Regarding the whole design in general, I used to have version that does not extend PSet with a slightly less nice syntax, but then moved to the current version because it seems to be supported (and also I found other examples).