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

Develop dqm 710pre2 v03 #3049

Closed

Conversation

webermat
Copy link
Contributor

Calls of functions of the DQMStore should have been removed by now to run safely in threaded DQM. Only exception is a DQMStore ->save() call if we run our code privately (switched off per default).

The only place where we still use DQMStore functionality is during the harvesting step, so hopefully all problematic issues have been solved.

We work with release CMSSW_7_1_0_pre2.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @webermat for CMSSW_7_1_X.

Develop dqm 710pre2 v03

It involves the following packages:

DQMOffline/JetMET

@ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please review it and eventually sign? Thanks.
@TaiSakuma, @nhanvtran this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@webermat
Copy link
Contributor Author

Looking through the comparison plots, I noticed additional plots which didn't appear when i ran locally. Reason: a boolean was defined but used uninitialized, I apologize for only noticing this so late.

@cmsbuild
Copy link
Contributor

Pull request #3049 was updated. @ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please check and sign again.

@cmsbuild
Copy link
Contributor

@deguio
Copy link
Contributor

deguio commented Apr 3, 2014

ciao @webermat
something I notice only now. the DataCertificationJetMET class is run in the harvesting and you have migrated it to the DQMEDAnalyzer interface. the migration doesn't involve the harvesting clients!

I have written it in red now: ;)
https://twiki.cern.ch/twiki/bin/viewauth/CMS/ThreadedDQM

Could you please make sure that only the modules run in DQM step1 have been migrated?

thanks,
F.

@deguio
Copy link
Contributor

deguio commented Apr 3, 2014

and I fear there is more. apparently both the JetAnalyzer and the MetAnalyzer are accessing thread-unsafe event setup object. they still need to be fixed. for a list have a look at the table in slide9:
https://indico.cern.ch/event/293016/session/7/contribution/63/material/slides/0.pdf
and
http://home.fnal.gov/~gartung/scan-build/esd2tlf.txt

I am not asking you to take care of those, but I have to keep the pull request pending for now. I am sorry about that. it is a huge transition the one we are facing. hopefully it will be smoother and smoother soon...

@webermat
Copy link
Contributor Author

webermat commented Apr 3, 2014

so the first comments have been taken care off (reverting back to the EDAnalyzer structure for the Harvesting modules). I am afraid we cannot take care of everything for the second item. I removed the call of the MagneticField, which we used to calculate the slope of tracks to the Calorimeter. Since we needed it only for JPTJets and these are to be removed, we don't need that call anymore.
The second call of the setup is the JetCorrector, which performs the jet energy correction; we for sure want to keep the jet energy corrections in.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2014

Pull request #3049 was updated. @ojeda, @danduggan, @rovere, @cmsbuild, @nclopezo, @deguio, @Degano can you please check and sign again.

@Dr15Jones
Copy link
Contributor

As long as a module is kept with the old interface it is safe to talk to thread unsafe EventSetup objects (i.e. JetCorrector). Now it happens the JetCorrector has been fixed since the last time we did the 'module to bad EventSetup' check.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2014

@deguio
Copy link
Contributor

deguio commented Apr 9, 2014

-1
superseded by #3262

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

Successfully merging this pull request may close these issues.

None yet

5 participants