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

fix crash in pre4 Assertion `seedCollection==nullptr' failed. #5022

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Aug 21, 2014

seedCollection needs to be reset to zero on a legal exit from the method, otherwise next call will give an assert

should fix crashes reported in https://hypernews.cern.ch/HyperNews/CMS/get/relval/3121.html
Tested with http://cms-project-relval.web.cern.ch/cms-project-relval/failures/CMSSW_7_2_0_pre4/standard/WElSkim2012D__RelVal_wEl2012D_Job1.tar.gz

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slava77 (Slava Krutelyov) for CMSSW_7_2_X.

fix crash in pre4 Assertion `seedCollection==nullptr' failed.

It involves the following packages:

RecoTracker/ConversionSeedGenerators

@nclopezo, @cmsbuild, @Degano, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @GiacomoSguazzoni, @rovere, @VinInn, @gpetruc, @cerati, @venturia 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.

@slava77
Copy link
Contributor Author

slava77 commented Aug 21, 2014

+1

for #5022 fe57c40

checked in pre4 that the assert is gone

@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 or unless it breaks tests.

@VinInn
Copy link
Contributor

VinInn commented Aug 21, 2014

Why the assert did not trigger on the usual Matrix?
(I introduced the assert just to make sure such a bug did not sneak in...)

@VinInn
Copy link
Contributor

VinInn commented Aug 21, 2014

Thanks Slava for the fix

@slava77
Copy link
Contributor Author

slava77 commented Aug 21, 2014

we don't have complex enough events in the matrix tests

the crash I was looking at was following the warning message from the previous event

%MSG-e TooManyClusters:   PhotonConversionTrajectorySeedProducerFromSingleLeg:photonConvTrajSeedFromSingleLeg  21-Aug-2014 17:38:32 CEST Run: 207454 Event: 1868
625186
Found too many clusters (28767), bailing out.

@VinInn
Copy link
Contributor

VinInn commented Aug 21, 2014

The old problem of testing all possible branch combinations in integration tests: notoriously impossible...
One needs synthetic, ad-hoc, unit test for that.

davidlange6 added a commit that referenced this pull request Aug 21, 2014
…720pre4assert-conversionOneLeg

fix crash in pre4 Assertion `seedCollection==nullptr' failed.
@davidlange6 davidlange6 merged commit 1921c89 into cms-sw:CMSSW_7_2_X Aug 21, 2014
@venturia
Copy link
Contributor

Hi,

for this specific kind of problem a possible way to "enhance" the rate of pathological events is to run in the test a special configuration where these thresholds are artificially low so that their are triggered more often. Or you could create a skim of odd events. I used to run it privately for the tracker with the purpose of monitoring their rate.

On Aug 21, 2014, at 22:01 , David Lange <notifications@github.commailto:notifications@github.com> wrote:

Merged #5022#5022.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5022#event-155753647.

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