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

TestDQMOnlineClient-beampixel_dqm_sourceclient had ERRORS #33356

Closed
silviodonato opened this issue Apr 7, 2021 · 16 comments · Fixed by #33378
Closed

TestDQMOnlineClient-beampixel_dqm_sourceclient had ERRORS #33356

silviodonato opened this issue Apr 7, 2021 · 16 comments · Fixed by #33378

Comments

@silviodonato
Copy link
Contributor

silviodonato commented Apr 7, 2021

Since CMSSW_11_3_X_2021-04-06-2300 we get a unit test failure in DQM/Integration

===== Test "TestDQMOnlineClient-beampixel_dqm_sourceclient" ====
+ [[ 1 -eq 0 ]]
+ [[ -z '' ]]
+ LOCAL_TEST_DIR=.
+ [[ -z '' ]]
+ CLIENTS_DIR=./src/DQM/Integration/python/clients
+ mkdir -p ./upload
+ cmsRun ./src/DQM/Integration/python/clients/beampixel_dqm_sourceclient-live_cfg.py unitTest=True
Querying DAS for files...
the query is file run=334393 dataset=/ExpressCosmics/Commissioning2019-Express-v1/FEVT lumi=1
DAS succeeded after 1 attempts 0
found files:  ['/store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/D0F052ED-9CA5-F547-BA73-2AA370D51AE8.root']
edmFileUtil --catalog file:/cvmfs/cms-ib.cern.ch/SITECONF/local/PhEDEx/storage.xml?protocol=xrootd --events /store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/D0F052ED-9CA5-F547-BA73-2AA370D51AE8.root | tail -n +9 | head -n -5 | awk '{ print $3 }'
the query is file run=334393 dataset=/ExpressCosmics/Commissioning2019-Express-v1/FEVT lumi=2
DAS succeeded after 1 attempts 0
found files:  ['/store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/FE21789A-F777-0B43-A1F5-E43F1FD52D19.root']
edmFileUtil --catalog file:/cvmfs/cms-ib.cern.ch/SITECONF/local/PhEDEx/storage.xml?protocol=xrootd --events /store/express/Commissioning2019/ExpressCosmics/FEVT/Express-v1/000/334/393/00000/FE21789A-F777-0B43-A1F5-E43F1FD52D19.root | tail -n +9 | head -n -5 | awk '{ print $3 }'
Got 2 files.
Loaded configuration file from: []
dqmRunConfig: cms.PSet(
    collectorHost = cms.untracked.string('127.0.0.1'),
    collectorPort = cms.untracked.int32(9190),
    type = cms.untracked.string('userarea')
)
----- Begin Fatal Exception 07-Apr-2021 03:14:31 CEST-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named ./src/DQM/Integration/python/clients/beampixel_dqm_sourceclient-live_cfg.py
Exception Message:
 unknown python problem occurred.
AttributeError: 'EDProducer' object has no attribute 'TkFilterParameters'

At:
  ./src/DQM/Integration/python/clients/beampixel_dqm_sourceclient-live_cfg.py(93): <module>

----- End Fatal Exception -------------------------------------------------

---> test TestDQMOnlineClient-beampixel_dqm_sourceclient had ERRORS

The origin of the error is here https://github.com/cms-sw/cmssw/blob/master/DQM/Integration/python/clients/beampixel_dqm_sourceclient-live_cfg.py#L93

The issue seems related to the change in RecoPixelVertexing/Configuration/python/RecoPixelVertexing_cff.py made by #31723 and already discussed https://github.com/cms-sw/cmssw/pull/31723/files#r510920848

@silviodonato
Copy link
Contributor Author

assign dqm

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

New categories assigned: dqm

@jfernan2,@andrius-k,@ahmad3213,@kmaeshima,@rvenditti,@ErnestaP you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2021

A new Issue was created by @silviodonato Silvio Donato.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@silviodonato
Copy link
Contributor Author

silviodonato commented Apr 7, 2021

The most trivial fix would be replacing
process.pixelVertices.TkFilterParameters.minPt = process.pixelTracksTrackingRegions.RegionPSet.ptMin
with
process.pixelVertices.PtMin = process.pixelTracksTrackingRegions.RegionPSet.ptMin
(see https://github.com/cms-sw/cmssw/blob/master/RecoPixelVertexing/PixelVertexFinding/python/PixelVertexes_cfi.py#L16)

but I'm not sure is what we want

cc @cms-sw/dqm-l2

@makortel
Copy link
Contributor

makortel commented Apr 7, 2021

Just to add that #31723 changed the import in RecoPixelVertexing/Configuration/python/RecoPixelVertexing_cff.py as

- from RecoVertex.PrimaryVertexProducer.OfflinePixel3DPrimaryVertices_cfi import *
+ from RecoPixelVertexing.PixelVertexFinding.PixelVertexes_cff import *

and therefore going from PrimaryVertexProducer to PixelVertexProducer.

@jfernan2
Copy link
Contributor

jfernan2 commented Apr 7, 2021

The most trivial fix would be replacing
process.pixelVertices.TkFilterParameters.minPt = process.pixelTracksTrackingRegions.RegionPSet.ptMin
with
process.pixelVertices.PtMin = process.pixelTracksTrackingRegions.RegionPSet.ptMin
(see https://github.com/cms-sw/cmssw/blob/master/RecoPixelVertexing/PixelVertexFinding/python/PixelVertexes_cfi.py#L16)

but I'm not sure is what we want

cc @cms-sw/dqm-l2

@mmusich @missirol would you know by chance?

@mmusich
Copy link
Contributor

mmusich commented Apr 7, 2021

Depends if you want to reproduce the past (then change back to the offline DA clustering-based vertexing) or buy the new configuration based on the divisive vertex finding (used online)...
Perhaps @dinardo and @gennai are better suited to answer this.

@missirol
Copy link
Contributor

missirol commented Apr 7, 2021

(@jfernan2 , I don't know these modules well, but Silvio's suggestion looks like the minimal consistent fix: .TkFilterParameters.minPt and .PtMin are the lower pT thresholds for the tracks used in the vertexing by these two different algorithms)

@dinardo
Copy link
Contributor

dinardo commented Apr 8, 2021

Dear @mmusich , @makortel , all, I originally wrote the module analyzer and the cfg file. The idea was to have an online "fast" beam monitor based only on the pixel information. As you pointed out different things have changed on the vertexing and I didn't keep track of all the changes. From what you wrote in your comments, I would go with the new feature, the "divisive vertex finding" (I guess this divisive vertex finding can also be used offline in case one wants to re-run this BeamPixel monitor tool (?))

@mmusich
Copy link
Contributor

mmusich commented Apr 8, 2021

Hi @dinardo,
the divisive vertex finder (as used for online applications, such as pixel vertexing @ HLT) is not new per se, it's just that the change included in PR #31723 (which is new) as explained in #33356 (comment) accidentally changed the meaning of process.pixelVertices in your configuration.
Perhaps you can clarify if you chose the offline (3D) vertex clustering instead of the 1D (z-only) clustering when writing the original configuration on purpose. If that's the case, I think we'd better change the client back to the original behaviour.

@gennai
Copy link
Contributor

gennai commented Apr 8, 2021

Well, if we want to monitor the beam spot (as it seems to be the case) we need the 3D vertexing.
Just a comment, if the code just monitor the beam spot position than it is very similar to what @ferencek client does for the HLT beamspot measurement. Maybe we can avoid doubling the resources and in case unify them?

Best,
S.

@dinardo
Copy link
Contributor

dinardo commented Apr 8, 2021

Hi @mmusich , ok it's clear. Then yes, we definitely need the offline (3D) vertex. How shall we proceed then?

@mmusich
Copy link
Contributor

mmusich commented Apr 8, 2021

. How shall we proceed then?

I can have a look if I can provide a quick fix. Stay tuned.

mmusich added a commit to mmusich/cmssw that referenced this issue Apr 8, 2021
@mmusich
Copy link
Contributor

mmusich commented Apr 8, 2021

see #33378.

mmusich added a commit to mmusich/cmssw that referenced this issue Apr 8, 2021
@jfernan2
Copy link
Contributor

jfernan2 commented Apr 9, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants