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 Candidate iterator #7617

Merged
merged 4 commits into from Feb 12, 2015

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Feb 8, 2015

Simplify implementation of Candidate's (component) iterator.
There is still a virtual call for each deference (and invocation of Candidate::end)
at least no memory-churn and no requirements on derived classes

No regression observed

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 8, 2015

A new Pull Request was created by @VinInn (Vincenzo Innocente) for CMSSW_7_4_X.

reimplement Candidate iterator

It involves the following packages:

DataFormats/Candidate
DataFormats/PatCandidates

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

@davidlange6
Copy link
Contributor

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 8, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 8, 2015

-1

Tested at: c43314b
I ran the usual tests and I found errors in the following unit tests:

---> test runtestTqafTopHitFit had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafTopJetCombination had ERRORS
---> test runtestTqafExamples had ERRORS
---> test runtestTqafTopKinFitter had ERRORS
---> test runtestPhysicsToolsPatAlgos had ERRORS
---> test runtestTqafTopEventProducers had ERRORS
---> test testJetMETCorrectionsType1MET had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7617/2447/summary.html

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

+1

for [#7617] c43314b

This PR involves minor code simplification of Candidate iterators.
It should not cause observable differences.

Code changes are satisfactory.

Jenkins tests CMSSW_7_4_X_2015-02-10-0200:
compilation, unit tests, matrix tests, static checks are all OK.

As expected, there are no significant differences compared to the baseline in the
following:

alternative-comparisons
(after stripping false positives like timer data)

and

validateJR

@monttj
Copy link
Contributor

monttj commented Feb 11, 2015

Do you have any clues why I am having this kind of error message (*) while compiling?
But the test is approved?

It is being tested at CMSSW_7_4_X_2015-02-11-0200.

()
Traceback (most recent call last):
File "/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc491/cms/cmssw-patch/CMSSW_7_4_X_2015-02-10-0200/src/FWCore/Utilities/scripts/edmCheckClassVersion", line 175, in
raise RuntimeError("failed to load library '"+options.library+"'")
RuntimeError: failed to load library 'libDataFormatsPatCandidatesCapabilities.so'
Suggestion: You can run 'scram build updateclassversion' to generate src/DataFormats/PatCandidates/src/classes_def_objects.xml.generated with updated ClassVersion
gmake: *
* [tmp/slc6_amd64_gcc491/src/DataFormats/PatCandidates/src/DataFormatsPatCandidates/libDataFormatsPatCandidatesCapabilities.so] Error 1
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@slava77
Copy link
Contributor

slava77 commented Feb 11, 2015

was the test build complete in your case? Jenkins completed tests including running PAT OK
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7617/2492/unitTests.log
so, I can only guess a problem with the local (re)build

@monttj
Copy link
Contributor

monttj commented Feb 11, 2015

Thank you for the feedback. Yes, it must be a local build problem that I could not understand so far.

davidlange6 added a commit that referenced this pull request Feb 12, 2015
@davidlange6 davidlange6 merged commit 6483017 into cms-sw:CMSSW_7_4_X Feb 12, 2015
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