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

Changing exceptions to LogErrors where appropriate #5803

Merged
merged 2 commits into from Oct 15, 2014

Conversation

rappoccio
Copy link
Contributor

Masking problem reported here :
https://hypernews.cern.ch/HyperNews/CMS/get/relval/3177.html
The new code will no longer throw Exception, but instead print to the LogError stream.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rappoccio for CMSSW_7_2_X.

Changing exceptions to LogErrors where appropriate

It involves the following packages:

CondFormats/JetMETObjects

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

// lep2 = pTrel2 + pLrel2
pTrel2 = lep2-pLrel2;
}
edm::LogError("JetCorrectionUncertainty")<<" not positive lepton-jet momentum: "<<lj2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm ... did not read the full file, but should this logError not be inside an "else" or something ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This is supposed to have an else. One moment.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@vadler
Copy link

vadler commented Oct 14, 2014

@ktf : Compilation currently fails with

c++: internal compiler error: Killed (program cc1plus)

even without the code changes introduced here.
I have tried CMSSW_7_2_X_2014-10-12-0200 and CMSSW_7_2_X_2014-10-09-1400.
73X is fine, thought.

@davidlt
Copy link
Contributor

davidlt commented Oct 14, 2014

@vadler what compiler? If 4.8, have you tried 4.9? I see this marked as "tests-approved". Could you provide full recipe for producing ICE (internal compiler error)?

@vadler
Copy link

vadler commented Oct 14, 2014

On LXPLUS:

setenv SCRAM_ARCH slc6_amd64_gcc481
cmsrel CMSSW_7_2_X_2014-10-12-0200
cd CMSSW_7_2_X_2014-10-12-0200/src
cmsenv
git cms-addpkg CondFormats/JetMETObjects
git checkout -b test-5803
git cms-merge-topic 5803
git checkout from-CMSSW_7_2_X_2014-10-12-0200
scram b -j20

I'm trying 491 now....

@vadler
Copy link

vadler commented Oct 14, 2014

Same thing with 491 :(
It seems to bee release cycle related, since 73X compiles fine.

@davidlt
Copy link
Contributor

davidlt commented Oct 14, 2014

@vadler all worked fine on cmsdev04. Could you point to exact lxplus machine?

@vadler
Copy link

vadler commented Oct 14, 2014

It indeed seems to be related to the machine (lxplus0115) and the -j20 option. It also worked fine for me without that or on a different machine.

@davidlt
Copy link
Contributor

davidlt commented Oct 14, 2014

Ah, I just now looked again at the message Killed (program cc1plus). It's probably OOM (out-of-memory) killer. You might be pushing machine too hard on memory side. It's something like 1GB per GCC instance.

@davidlange6
Copy link
Contributor

@vadler - is this one ready for you? (and the same for 70x)

@vadler
Copy link

vadler commented Oct 15, 2014

+1
Tested with customized WF 4.73 (reproducing the same error as in https://hypernews.cern.ch/HyperNews/CMS/get/relval/3177.html at CERN) in CMSSW_7_2_X_2014-10-12-0200.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). @nclopezo can you please take care of it?

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

6 participants