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

Reimplement SimpleJetCorrector::correctionBin for ROOT 6.04.00 #9289

Merged
merged 1 commit into from
May 29, 2015

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented May 27, 2015

ROOT6 (master, pre-6.04.00) has a new implemention for TFormula. For
thread-safety we are making copies of TFormula, because we need to
modify it by changing the parameters. Copies are no more needed for
ROOT 6.04.00 and instead

Double_t EvalPar(const Double_t *, const Double_t *) const

can be used to evalute TFormula without modification.

Without this change CMSSW spends most of it's cpu time in TFormula ctor
and memory consumption (due to string allocations) goes up by 2-2.5X.

ROOT6 commit, which made EvalPar thread-safe:
bcfb68afac1db25d824d35557493d0f3c941b332

Tested with CMSSW_7_5_DEVEL_X_2015-05-26-2300 (incl. ROOT6
master branch), resource usage compared with 4.63 workflow step3 (RECO).
All within expected behavior.

Without this pull request CMSSW + ROOT 6.04.00 most likely would force
build machine to go offline (due to memory usage).

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

ROOT6 (master, pre-6.04.00) has a new implemention for TFormula. For
thread-safety we are making copies of TFormula, because we need to
modify it by changing the parameters. Copies are no more needed for
ROOT 6.04.00 and instead

    Double_t EvalPar(const Double_t *, const Double_t *) const

can be used to evalute TFormula without modification.

Without this change CMSSW spends most of it's cpu time in TFormula ctor
and memory consumption (due to string allocations) goes up by 2-2.5X.

ROOT6 commit, which made `EvalPar` thread-safe:
bcfb68afac1db25d824d35557493d0f3c941b332

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@davidlt
Copy link
Contributor Author

davidlt commented May 27, 2015

I scheduled the tests in Jenkins.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlt for CMSSW_7_5_X.

Reimplement SimpleJetCorrector::correctionBin for ROOT 6.04.00

It involves the following packages:

CondFormats/JetMETObjects

@ggovi, @vadler, @cmsbuild, @nclopezo, @monttj can you please review it and eventually sign? Thanks.
@TaiSakuma, @ahinzmann, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @apfeiffer1, @schoef, @mariadalfonso 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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@Dr15Jones
Copy link
Contributor

hold

@Dr15Jones
Copy link
Contributor

Please test

@Dr15Jones
Copy link
Contributor

Remove hold

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@davidlt
Copy link
Contributor Author

davidlt commented May 27, 2015

In was verified in ROOT planing meeting that EvalPar is thread-safe on TFormula.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@apfeiffer1
Copy link
Contributor

+1

@davidlt
Copy link
Contributor Author

davidlt commented May 28, 2015

ping^1, this is an important and the last piece. Without this build machines will have hard time due to high memory consumption.

@davidlt
Copy link
Contributor Author

davidlt commented May 29, 2015

Judging by http://cms-sw.github.io/stats/pending-prs.html?category=analysis L2 managers for analysis are not available for some time.

@davidlange6
Copy link
Contributor

I was more worried by the DB signature given the change to CondFormats. ok - since everything changed is transient, I'll merge and let people complain given the small changes

davidlange6 added a commit that referenced this pull request May 29, 2015
Reimplement SimpleJetCorrector::correctionBin for ROOT 6.04.00
@davidlange6 davidlange6 merged commit dd1b6d7 into cms-sw:CMSSW_7_5_X May 29, 2015
@davidlt
Copy link
Contributor Author

davidlt commented May 29, 2015

I am not sure how exactly to test CondFormats. I only tested RECO on 100 events and produced 0 differences between normal IB and IB + ROOT6 master + this change.

If that's a correct test. I will repeat it once IB is available on more workflows.

@rappoccio
Copy link
Contributor

You can run the miniAOD tests, those should test the CondFormats for JEC's sufficiently. Alternatively the DQM workflows also use this without PAT.

@davidlange6
Copy link
Contributor

to be clear, I was worried not for running-without-crashing tests but db schema issues (which there are none)

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.

6 participants