Clean Obsolete protection against C++11 syntax #12672

VinInn · 2015-12-04T16:52:09Z

purely syntactic. In principle even the post-processed code shall be identical

cmsbuild · 2015-12-04T16:52:41Z

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

It involves the following packages:

Alignment/ReferenceTrajectories
CalibFormats/SiStripObjects
DataFormats/CSCRecHit
DataFormats/CaloTowers
DataFormats/Candidate
DataFormats/GEMRecHit
DataFormats/GeometryCommonDetAlgo
DataFormats/GeometrySurface
DataFormats/GeometryVector
DataFormats/Math
DataFormats/TrackReco
DataFormats/TrackerRecHit2D
DataFormats/TrackingRecHit
DataFormats/TrajectorySeed
DataFormats/VertexReco
Geometry/CaloGeometry
Geometry/CaloTopology
Geometry/CommonDetUnit
Geometry/CommonTopologies
Geometry/EcalAlgo
Geometry/HGCalGeometry
Geometry/TrackerGeometryBuilder
RecoCaloTools/Navigation
RecoMuon/TransientTrackingRecHit
RecoPixelVertexing/PixelTrackFitting
RecoTracker/MeasurementDet
RecoTracker/TkDetLayers
RecoTracker/TkMSParametrization
RecoTracker/TkNavigation
RecoTracker/TkSeedGenerator
RecoTracker/TkTrackingRegions
RecoTracker/TransientTrackingRecHit
RecoVertex/KinematicFit
RecoVertex/KinematicFitPrimitives
RecoVertex/PrimaryVertexProducer
RecoVertex/VertexPrimitives
RecoVertex/VertexTools
TrackPropagation/NavGeometry
TrackPropagation/RungeKutta
TrackPropagation/SteppingHelixPropagator
TrackingTools/DetLayers
TrackingTools/GeomPropagators
TrackingTools/GsfTools
TrackingTools/GsfTracking
TrackingTools/KalmanUpdators
TrackingTools/MaterialEffects
TrackingTools/MeasurementDet
TrackingTools/PatternTools
TrackingTools/TrackFitters
TrackingTools/TrajectoryState
TrackingTools/TransientTrack
TrackingTools/TransientTrackingRecHit

@civanch, @diguida, @cvuosalo, @ianna, @mdhildreth, @monttj, @cmsbuild, @franzoni, @Dr15Jones, @cerminar, @slava77, @vadler, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @abbiendi, @argiro, @tlampen, @threus, @venturia, @battibass, @makortel, @jhgoh, @cerati, @trocino, @rociovilar, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @mschrode, @mmusich, @dgulhan, @gpetruc, @istaslis, @bachtis this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

+1|approve[d]|sign[ed]: L1/L2's to approve it
-1|reject[ed]: L1/L2's to reject it
assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
hold: L1/all L2's/release manager to mark it as on hold
unhold: L1/user who put this PR on hold
merge: L1/release managers to merge this request
[@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
[@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

VinInn · 2015-12-04T16:54:46Z

@cmsbuild, please test

cmsbuild · 2015-12-04T16:55:15Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10145/console

slava77 · 2015-12-04T17:12:58Z

DataFormats/GeometrySurface/interface/ReferenceCounted.h


-      const BasicReferenceCounted& operator=( const BasicReferenceCounted& ) {
+      BasicReferenceCounted& operator=( const BasicReferenceCounted& ) {


why did the const go away?
is it generally const-safe when used outside

Removing const is the proper thing to do since that is really the signature expected by C++. Besides, the member function isn't const anyway.

The syntax of an assign operator is this one.
All this exists because reference counted objects may also be used as normal objects.
Otherwise we can simply delete copy and assignment if used only in refcoutingpointers...

slava77 · 2015-12-04T17:17:51Z

I suppose no regressions were observed.
Vincenzo, did you check locally?

cmsbuild · 2015-12-04T22:41:30Z

-1

Tested at: daa30c3
I found errors in the following unit tests:

---> test testRecoMETMETProducers had ERRORS
---> test runtestSimCalorimetryHGCalSimProducers had ERRORS

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

cmsbuild · 2015-12-04T23:48:45Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12672/10145/summary.html

VinInn · 2015-12-05T08:49:19Z

not me

Exception Message:
Failed to open the file 'ttbarForMetTests.root'
   Additional Info:
      [a] Input file file:ttbarForMetTests.root could not be opened.
      [b] open() failed with system error 'No such file or directory' (error code 2)
----- End Fatal Exception -------------------------------------------------
Failure using recoMET_genMet_cfg.py: status 84
status = 21504

---> test testRecoMETMETProducers had ERRORS

---- Begin Fatal Exception 04-Dec-2015 21:57:03 CET-----------------------
An exception of category 'Filling of conditions failed' occurred while
   [0] Processing run: 1
   [1] Calling global beginRun for module MixingModule/'mix'
   [2] Using EventSetup component HcalHardcodeCalibrations/'es_hardcode' to make data HcalMCParams/'' in record HcalMCParamsRcd
Exception Message:
 no valid filling possible for Conditions of type HcalMCParams for DetId (0x4a021483) (HcalTrigTower v1: -41,3)
----- End Fatal Exception -------------------------------------------------
terminate called after throwing an instance of 'cms::Exception'
  what():  An exception of category 'Assert' occurred.
Exception Message:
Geant4 is still alive, master thread state must be set to EndRun before Destruct

/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2015-12-04-1100/src/SimCalorimetry/HGCalSimProducers/test/runtests.sh: line 5:  5754 Aborted                 (core dumped) cmsRun ${LOCAL_TEST_DIR}/testHGCalDigi_cfg.py
Failure using testHGCalDigi_cfg.py: status 134
status = 34304

---> test runtestSimCalorimetryHGCalSimProducers had ERRORS

I have not detected any regression

VinInn · 2015-12-05T08:51:20Z

@davidlange6 , I propose to merge as is

slava77 · 2015-12-05T11:51:35Z

+1

for #12672 daa30c3

changes are in line with the PR description
jenkins tests pass (ignoring the unit tests failing in the IB for the HGCAL and previously seen failure in the MET tests). Comparisons with the baseline show no differences.

We should probably have an IB with this PR first before just pushing in a pre-release.

civanch · 2015-12-05T15:49:49Z

+1

Clean Obsolete protection against C++11 syntax

davidlange6 · 2015-12-07T08:14:01Z

@VinInn - only because I do not see other obvious causes... any way this PR can lead to this error in the IBs:
@slava77, @cvuosalo

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc493/CMSSW_8_0_X_2015-12-06-1100/pyRelValMatrixLogs/run/1307.0_CosmicsSPLoose_UP15+CosmicsSPLoose_UP15INPUT+DIGIHAL+RECOHAL+ALCAHAL+HARVESTHAL/step3_CosmicsSPLoose_UP15+CosmicsSPLoose_UP15INPUT+DIGIHAL+RECOHAL+ALCAHAL+HARVESTHAL.log

VinInn · 2015-12-07T08:31:01Z

It passed the Matrix... is it the only error? do not see any obvious reason...

davidlange6 · 2015-12-07T08:49:51Z

i don’t see any reason either… but thought I’d ask - this is the only new error - maybe it was a glitch…

On Dec 7, 2015, at 9:31 AM, Vincenzo Innocente notifications@github.com wrote:

It passed the Matrix... is it the only error? do not see any obvious reason...

—
Reply to this email directly or view it on GitHub.

Dr15Jones · 2015-12-07T15:41:35Z

I looked at the configuration for the failing workflow. A Producer labelled 'ecalFixedAlphaBetaFitUncalibRecHit' is in the configuration but I don't think it appears on any of the run paths since no module with that label is created in the job (as determined by using Tracer service).

I noticed this configuration use the 'reconstructonCosmics' sequence. Maybe that one has a problem?

VinInn added 5 commits December 3, 2015 15:57

clean macro, remove obsolete files

8641e54

add asin

676ab7c

more cleanup

c23a3fa

fix refcounting assignment

47c3e9c

fix refcounting assignment

daa30c3

cmsbuild added this to the Next CMSSW_8_0_X milestone Dec 4, 2015

cmsbuild added reconstruction-pending simulation-pending analysis-pending alca-pending geometry-pending pending-signatures tests-pending orp-pending comparison-pending labels Dec 4, 2015

cmsbuild added tests-started and removed tests-pending labels Dec 4, 2015

slava77 reviewed Dec 4, 2015
View reviewed changes

cmsbuild added tests-rejected and removed tests-started labels Dec 4, 2015

cmsbuild added comparison-available and removed comparison-pending labels Dec 4, 2015

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Dec 5, 2015

cmsbuild added geometry-approved simulation-approved and removed geometry-pending simulation-pending labels Dec 5, 2015

davidlange6 added a commit that referenced this pull request Dec 6, 2015

Merge pull request #12672 from VinInn/CleanMath80

c10c397

Clean Obsolete protection against C++11 syntax

davidlange6 merged commit c10c397 into cms-sw:CMSSW_8_0_X Dec 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean Obsolete protection against C++11 syntax #12672

Clean Obsolete protection against C++11 syntax #12672

VinInn commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

VinInn commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

slava77 Dec 4, 2015

Dr15Jones Dec 4, 2015

VinInn Dec 4, 2015

slava77 Dec 4, 2015

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

VinInn commented Dec 5, 2015

VinInn commented Dec 5, 2015

slava77 commented Dec 5, 2015

civanch commented Dec 5, 2015

davidlange6 commented Dec 7, 2015

VinInn commented Dec 7, 2015

davidlange6 commented Dec 7, 2015

Dr15Jones commented Dec 7, 2015


		const BasicReferenceCounted& operator=( const BasicReferenceCounted& ) {
		BasicReferenceCounted& operator=( const BasicReferenceCounted& ) {

Clean Obsolete protection against C++11 syntax #12672

Clean Obsolete protection against C++11 syntax #12672

Conversation

VinInn commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

VinInn commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

slava77 Dec 4, 2015

Choose a reason for hiding this comment

Dr15Jones Dec 4, 2015

Choose a reason for hiding this comment

VinInn Dec 4, 2015

Choose a reason for hiding this comment

slava77 Dec 4, 2015

Choose a reason for hiding this comment

slava77 commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

cmsbuild commented Dec 4, 2015

VinInn commented Dec 5, 2015

VinInn commented Dec 5, 2015

slava77 commented Dec 5, 2015

civanch commented Dec 5, 2015

davidlange6 commented Dec 7, 2015

VinInn commented Dec 7, 2015

davidlange6 commented Dec 7, 2015

Dr15Jones commented Dec 7, 2015