Pixel raw2 digi #1995

VinInn · 2014-01-11T13:48:48Z

sped up of Pixel Raw2Digi
more details in https://its.cern.ch/jira/browse/CMSHLT-34

a factor 1.64
in principle more could be saved if the detId were produced in order
(half of the time is spent in copying backward DetSets in the final product (DetSetVector)

cmsbuild · 2014-01-11T13:53:04Z

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

Pixel raw2 digi

It involves the following packages:

CondFormats/SiPixelObjects
DataFormats/SiPixelDigi
EventFilter/SiPixelRawToDigi

@apfeiffer1, @diguida, @civanch, @mdhildreth, @cmsbuild, @anton-a, @thspeer, @rcastello, @slava77, @ggovi, @Degano, @nclopezo 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.
@ktf you are the release manager for this.

cmsbuild · 2014-01-11T14:58:04Z

Pull request #1995 was updated. @apfeiffer1, @diguida, @civanch, @mdhildreth, @cmsbuild, @anton-a, @Dr15Jones, @rcastello, @slava77, @ggovi, @Degano, @ktf, @thspeer, @nclopezo can you please check and sign again.

VinInn · 2014-01-11T14:59:28Z

using emplace and =move() (instead of swap) removed all copy_backward etc
timing not affected at all! most probably is just memory access

cmsbuild · 2014-01-11T16:08:02Z

Pull request #1995 was updated. @apfeiffer1, @diguida, @civanch, @mdhildreth, @cmsbuild, @anton-a, @Dr15Jones, @rcastello, @slava77, @ggovi, @Degano, @ktf, @thspeer, @nclopezo can you please check and sign again.

civanch · 2014-01-13T09:30:44Z

+1

cmsbuild · 2014-01-13T09:33:04Z

Pull request #1995 was updated. @apfeiffer1, @diguida, @cmsbuild, @anton-a, @Dr15Jones, @rcastello, @slava77, @ggovi, @Degano, @ktf, @thspeer, @nclopezo can you please check and sign again.

rcastello · 2014-01-13T09:55:57Z

+1

Dr15Jones · 2014-01-13T12:53:56Z

CondFormats/SiPixelObjects/src/SiPixelFedCablingMap.cc

 using namespace sipixelobjects;

+
+void SiPixelFedCablingMap::initializeRocs() {
+  std::cout << "initialize PixelRocs" << std::endl;


Please remove the printout or replace it with MessageLogger.

On 13 Jan, 2014, at 1:54 PM, Chris Jones notifications@github.com wrote:

In CondFormats/SiPixelObjects/src/SiPixelFedCablingMap.cc:

using namespace sipixelobjects;

+void SiPixelFedCablingMap::initializeRocs() {

std::cout << "initialize PixelRocs" << std::endl;

Please remove the printout or replace it with MessageLogger.

sure, was just to make sure it was called…
v.

Dr15Jones · 2014-01-13T13:01:07Z

DataFormats/Common/interface/DetSet.h

@@ -51,7 +51,7 @@

    DetSet<T> & operator=(DetSet<T> && rh)  noexcept {
      id = rh.id;
-      data.swap(rh.data);
+      data = std::move(rh.data);


@VinInn Vincenzo, are there any other classes which are elements of DetSet which which do not have move op=? I ask since if there are you probably made a pessimistic optimization replacing a swap with a copy. I think the standard has some meta programming checks for the various move functions. Not 100% certain of that though.

On 13 Jan, 2014, at 2:01 PM, Chris Jones notifications@github.com wrote:

In DataFormats/Common/interface/DetSet.h:

@@ -51,7 +51,7 @@

DetSet<T> & operator=(DetSet<T> && rh) noexcept { id = rh.id;

data.swap(rh.data);

data = std::move(rh.data);

@VinInn Vincenzo, are there any other classes which are elements of DetSet which which do not have move op=?
I think DetSet is no used only by PixelDigi. Everybody else uses DeSetNew.
I ask since if there are you probably made a pessimistic optimization replacing a swap with a copy.
Havent seen any regression in Reco. If they exist, need to be fixed. (btw since each DetSet is sorted duing event put if T is not movable will show up there more than here)
I think the standard has some meta programming checks for the various move functions.
Indeed. the nastier being missing “noexcept”

v.

On 13 Jan, 2014, at 2:01 PM, Chris Jones notifications@github.com wrote:

In DataFormats/Common/interface/DetSet.h:

@@ -51,7 +51,7 @@

DetSet<T> & operator=(DetSet<T> && rh) noexcept { id = rh.id;

data.swap(rh.data);

data = std::move(rh.data);

@VinInn Vincenzo, are there any other classes which are elements of DetSet which which do not have move op=?
http://cmslxr.fnal.gov/lxr/search?v=CMSSW_7_1_X_2014-01-12-0200&filestring=classes_def.xml&string=DetSetVector
apparently only some FP420 stuff (what is it anyway? ah TOTEM…)

On 13 Jan, 2014, at 2:11 PM, Vincenzo Innocente vincenzo.innocente@cern.ch wrote:

On 13 Jan, 2014, at 2:01 PM, Chris Jones notifications@github.com wrote:

In DataFormats/Common/interface/DetSet.h:

@@ -51,7 +51,7 @@

DetSet<T> & operator=(DetSet<T> && rh) noexcept { id = rh.id;

data.swap(rh.data);

data = std::move(rh.data);

@VinInn Vincenzo, are there any other classes which are elements of DetSet which which do not have move op=?
http://cmslxr.fnal.gov/lxr/search?v=CMSSW_7_1_X_2014-01-12-0200&filestring=classes_def.xml&string=DetSetVector
apparently only some FP420 stuff (what is it anyway? ah TOTEM…)
they are movable

v.

On 13 Jan, 2014, at 2:01 PM, Chris Jones notifications@github.com wrote:

In DataFormats/Common/interface/DetSet.h:

@@ -51,7 +51,7 @@

DetSet<T> & operator=(DetSet<T> && rh) noexcept { id = rh.id;

data.swap(rh.data);

data = std::move(rh.data);

@VinInn Vincenzo, are there any other classes which are elements of DetSet which which do not have move op=? I ask since if there are you probably made a pessimistic optimization replacing a swap with a copy. I think the standard has some meta programming checks for the various move functions. Not 100% certain of that though.
and btw a vector is movable even if its elements are not movable
see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52591 for instance
v.

cmsbuild · 2014-01-13T14:25:41Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1995/1992/summary.html

cmsbuild · 2014-01-13T14:28:12Z

Pull request #1995 was updated. @apfeiffer1, @diguida, @civanch, @mdhildreth, @anton-a, @Dr15Jones, @rcastello, @slava77, @ggovi, @ktf, @thspeer can you please check and sign again.

Dr15Jones · 2014-01-13T14:44:10Z

+1

civanch · 2014-01-14T08:37:03Z

+1

rcastello · 2014-01-21T09:35:03Z

+1

anton-a · 2014-01-23T21:40:47Z

looking into it

anton-a · 2014-01-23T22:35:47Z

merged in CMSSW_7_0_X_2014-01-23-0200
fails to compile:
CMSSW_7_0_X_2014-01-23-0200_pr1995/src/CondCore/SiPixelPlugins/src/plugin.cc:38:77: error: expected constructor, destructor, or type conversion before ';' token
REGISTER_PLUGIN_INIT(SiPixelFedCablingMapRcd,SiPixelFedCablingMap, InitRocs);

VinInn · 2014-01-24T07:51:51Z

And how happens that worked in
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1995/1992/summary.html
???

VinInn · 2014-01-24T08:31:34Z

innocent@vinavx2 ~]$ cd /home/vin/CMSSW_7_0_X_2014-01-23-0200/src/
[innocent@vinavx2 src]$ grep -r REGISTER_PLUGIN_INIT *
CondCore/SiPixelPlugins/src/plugin.cc:REGISTER_PLUGIN_INIT(SiPixelFedCablingMapRcd,SiPixelFedCablingMap, InitRocs);
build.log: REGISTER_PLUGIN_INIT(SiPixelFedCablingMapRcd,SiPixelFedCablingMap, InitRocs);
[innocent@vinavx2 src]$ cmsenv
[innocent@vinavx2 src]$ grep -r REGISTER_PLUGIN_INIT $CMSSW_RELEASE_BASE/src/*
[innocent@vinavx2 src]$ cd ../../CMSSW_7_0_X_2014-01-14-0200/
[innocent@vinavx2 CMSSW_7_0_X_2014-01-14-0200]$ cmsenv
[innocent@vinavx2 CMSSW_7_0_X_2014-01-14-0200]$ grep -r REGISTER_PLUGIN_INIT $CMSSW_RELEASE_BASE/src/*
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-14-0200/src/CondCore/CalibPlugins/src/plugin.cc:REGISTER_PLUGIN_INIT(ExEfficiencyRcd, condex::Efficiency, InitEfficiency );
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-14-0200/src/CondCore/ESSources/interface/registration_macros.h:#define ONLY_REGISTER_PLUGIN_INIT(record_,type_, initializer_)
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-14-0200/src/CondCore/ESSources/interface/registration_macros.h:#define REGISTER_PLUGIN_INIT(record_, type_, initializer_)
/afs/cern.ch/cms/sw/ReleaseCandidates/vol1/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-14-0200/src/CondCore/ESSources/interface/registration_macros.h:ONLY_REGISTER_PLUGIN_INIT(record_, type_, initializer_ )

the macro il gone!
great...

ktf · 2014-01-24T08:32:34Z

Apparently the macro in question got removed by @ggovi in aabce96 . This is last time I approve "orthogonal" DB changes last minute… :-/ Giacomo, any idea how we can (quickly) fix this?

VinInn · 2014-01-24T08:34:17Z

also from the test
src/CondCore/CalibPlugins/src/plugin.cc:REGISTER_PLUGIN_INIT(ExEfficiencyRcd, convex::Efficiency, InitEfficiency );

easy life, isn't it?
does not compile? remove the test!

VinInn · 2014-01-24T08:36:18Z

How the merge succeeded ? where is the corresponding pull request?
I think this is a failure of our integration management: I arrive with an obsolete version of the whole cmssw, pull request, approve. bang!
no regression as everything works as it was already working ages ago....
with cvs this did not happen has the HEAD was the HEAD, one could not commit an old branch a top of the HEAD

apfeiffer1 · 2014-01-24T09:43:32Z

strange, the IB in question (CMSSW_7_0_X_2014-01-23-0200/) where you do not
see the macro reports no build errors whatsoever. Did you rebase your
branch onto this one so you get all the latest updates ?

Thanks,
cheers, andreas

VinInn · 2014-01-24T09:46:30Z

of douse does not report any error: the pullreguest form giacomo has erased everything was done in 1665!
so back to origin, does work!

in any case in CMSSW_7_0_X_2014-01-23-0200 basic tests of condcore as in PopCon/test do not work anyone
this is not normal
cmscond_list_iov -c sqlite_file:pop_test.db -t Example_tag1 -s

*** Break *** segmentation violation

===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================
#0  0x0000003267eac8be in waitpid () from /lib64/libc.so.6
#1  0x0000003267e3e909 in do_system () from /lib64/libc.so.6
#2  0x00007f64883aeddf in TUnixSystem::StackTrace() () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-23-0200/external/slc6_amd64_gcc481/lib/libCore.so
#3  0x00007f64883b08dc in TUnixSystem::DispatchSignals(ESignals) () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-23-0200/external/slc6_amd64_gcc481/lib/libCore.so
#4  <signal handler called>
#5  0x000000000040ddc4 in cond::ListIOVUtilities::execute() ()
#6  0x00007f648c763045 in cond::Utilities::run(int, char**) () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-23-0200/lib/slc6_amd64_gcc481/libCondCoreUtilities.so
#7  0x000000000040c07a in main ()
===========================================================


The lines below might hint at the cause of the crash.
If they do not help you then please submit a bug report at
http://root.cern.ch/bugs. Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.
===========================================================
#5  0x000000000040ddc4 in cond::ListIOVUtilities::execute() ()
#6  0x00007f648c763045 in cond::Utilities::run(int, char**) () from /afs/cern.ch/cms/sw/ReleaseCandidates/vol0/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_0_X_2014-01-23-0200/lib/slc6_amd64_gcc481/libCondCoreUtilities.so
#7  0x000000000040c07a in main ()
=======================================================

apfeiffer1 · 2014-01-24T09:51:13Z

Ciao Vincenzo,

in any case in CMSSW_7_0_X_2014-01-23-0200 basic tests of condcore as in

PopCon/test do not work anyone

strange, none of the slc5/scl6 IBs report any error for this IB:

https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/slc5_amd64_gcc481/www/thu/7.0-thu-02/CMSSW_7_0_X_2014-01-23-0200/

https://cmssdt.cern.ch/SDT/cgi-bin/showBuildLogs.py/slc6_amd64_gcc481/www/thu/7.0-thu-02/CMSSW_7_0_X_2014-01-23-0200/

which platform are you using ?

Can you please make the sqlite_file:pop_test.db available in your public
AFS so we can have a look ?

Thanks,
cheers, andreas

ggovi · 2014-01-24T09:57:15Z

Not sure to understand what is an "orthogonal" change. Anyhow, aabce96 is one month old, not really "a last minute". Anyhow,

with the last IB:

/build/gg/CMSSW_7_1_X_2014-01-24-0200/src $ cmscond_list_iov -c sqlite_file:runinfo_31X_mc -t runinfo_31X_mc
========================================================
Tag: runinfo_31X_mc
========================================================
OID: 0000-00000000
Scope: Tag
Description:
TimeType: runnumber
Since Till Payload OID Payload Class
-------------------- -------------------- ------------- -------------
1 4294967295 0001-00000000 RunInfo

So, vincenzo please send me your file, i'll try to reproduce your crash.

VinInn · 2014-01-24T09:59:17Z

this is what I did

cmsrel CMSSW_7_0_X_2014-01-23-0200
cd CMSSW_7_0_X_2014-01-23-0200/src
cmsenv
cp -r $CMSSW_RELEASE_BASE/src/CondCore .
cd CondCore/PopCon/test/
mkdir EffTest
pushd EffTest
cmsRun $LOCALRT/src/CondCore/PopCon/test/PopConEffExample.py
sqlite3 pop_test.db .tables
 cmscond_list_iov -c sqlite_file:pop_test.db -t Example_tag1 -s

repeat with
CMSSW_7_0_X_2014-01-14-0200/

ggovi · 2014-01-24T10:14:42Z

Vincenzo, I can explain you what is going on. The test you are running is creating a condition DB with the new format. The command cmscond_list_iov is able to operate only on the OLD format DB. Anyhow, I agree that the command should rather complain in a "polite" way and not crash. I'll provide a fix for this.

VinInn · 2014-01-24T10:30:34Z

git cms-merge-topic VinInn:PixelRaw2Digi
git cms-merge-topic ktf:bring-back-initializers

seems to work
compiles
I reverted last commit

--- a/CondFormats/SiPixelObjects/src/SiPixelFedCablingMap.cc
+++ b/CondFormats/SiPixelObjects/src/SiPixelFedCablingMap.cc
@@ -11,7 +11,7 @@ using namespace sipixelobjects;


 void SiPixelFedCablingMap::initializeRocs() {
-  //  std::cout << "initialize PixelRocs" << std::endl;
+  std::cout << "initialize PixelRocs" << std::endl;
   for (auto & v : theMap) v.second.initFrameConversion();

run
l
410 DQMStore::DQMStore
24-Jan-2014 11:19:52 CET Initiating request to open file file:run207515_lumi183.root
24-Jan-2014 11:19:52 CET Initiating request to open file file:run207515_lumi183.root
24-Jan-2014 11:19:52 CET Successfully opened file file:run207515_lumi183.root
24-Jan-2014 11:19:52 CET Successfully opened file file:run207515_lumi183.root
initialize PixelRocs
MSLayersKeeperX0DetLayer LAYERS:
24-Jan-2014 11:20:36 CET Closed file file:run207515_lumi183.root
24-Jan-2014 11:20:36 CET Closed file file:run207515_lumi183.root

the printout shows up...

I propose to integrate ktf:bring-back-initializers asap

VinInn · 2014-01-24T11:26:29Z

@anton-a : pull request #2158 restore what deleted (accidentally).
When integrated reco test can start again.
sorry for the mess

anton-a · 2014-01-26T13:50:29Z

@Degano, @nclopezo can you please run jenkins. Looks like #2158 is included.

cmsbuild · 2014-01-27T14:21:05Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1995/31/summary.html

apfeiffer1 · 2014-01-28T10:48:20Z

+1

anton-a · 2014-01-31T16:28:00Z

There are some failed track comparisons between the merged and baseline versions in jenkins, although no regression is expected. This behavior could not be reproduced in tests based on two different IB's - the offending distributions were identical for IB and IB+pr1995.

ktf · 2014-01-31T16:33:28Z

@Degano / @nclopezo , can you please restart the tests?

anton-a · 2014-02-03T20:20:42Z

@Degano, @nclopezo can you please run jenkins. I'd like to see if the comparisons are in agreement with my observations and close this item.

cmsbuild · 2014-02-04T10:32:35Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1995/14/summary.html

anton-a · 2014-02-04T14:12:19Z

+1
fea4693
tested in CMSSW_7_0_X_2014-01-25-0200 and CMSSW_7_1_X_2014-01-27-0200
no differences in related reconstruction

cmsbuild · 2014-02-04T14:12:48Z

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

cmsbuild · 2014-02-04T16:55:59Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1995/3/summary.html

davidlange6 · 2014-02-04T19:53:11Z

move to 71x please. (should go in if it merges)

ktf · 2014-02-05T12:06:52Z

Closing this for the moment. Will reopen when back-porting.

VinInn added 4 commits January 10, 2014 19:34

try to decode ROC once

1f25a65

skip if error

f92efe7

inline init, remove multiple storage of digis

cfc3e2c

use hashmap

46ef192

use emplace and =move

0bb3259

fetch fed once

375bb27

VinInn added 3 commits January 11, 2014 17:55

removed a bunch of global variables. verify roc-number inline

e31341e

removed another bunch of global variables

f934b6f

made PixelRoc thread safe. it is also faster now

54b3ebf

Dr15Jones reviewed Jan 13, 2014
View reviewed changes

remove cout

fea4693

Dr15Jones reviewed Jan 13, 2014
View reviewed changes

ktf mentioned this pull request Feb 4, 2014

Pixel raw2 digi #2297

Merged

ktf closed this Feb 5, 2014

nclopezo modified the milestones: CMSSW_7_0_0, CMSSW_7_0_0_pre13 Feb 7, 2014

VinInn deleted the PixelRaw2Digi branch July 13, 2016 13:45

Pixel raw2 digi #1995

Pixel raw2 digi #1995

Conversation

VinInn commented Jan 11, 2014

cmsbuild commented Jan 11, 2014

cmsbuild commented Jan 11, 2014

VinInn commented Jan 11, 2014

cmsbuild commented Jan 11, 2014

civanch commented Jan 13, 2014

cmsbuild commented Jan 13, 2014

rcastello commented Jan 13, 2014

Dr15Jones Jan 13, 2014

Choose a reason for hiding this comment

VinInn Jan 13, 2014

Choose a reason for hiding this comment

Dr15Jones Jan 13, 2014

Choose a reason for hiding this comment

VinInn Jan 13, 2014

Choose a reason for hiding this comment

VinInn Jan 13, 2014

Choose a reason for hiding this comment

VinInn Jan 13, 2014

Choose a reason for hiding this comment

VinInn Jan 13, 2014

Choose a reason for hiding this comment

cmsbuild commented Jan 13, 2014

cmsbuild commented Jan 13, 2014

Dr15Jones commented Jan 13, 2014

civanch commented Jan 14, 2014

rcastello commented Jan 21, 2014

anton-a commented Jan 23, 2014

anton-a commented Jan 23, 2014

VinInn commented Jan 24, 2014

VinInn commented Jan 24, 2014

ktf commented Jan 24, 2014

VinInn commented Jan 24, 2014

VinInn commented Jan 24, 2014

apfeiffer1 commented Jan 24, 2014

VinInn commented Jan 24, 2014

apfeiffer1 commented Jan 24, 2014

ggovi commented Jan 24, 2014

VinInn commented Jan 24, 2014

ggovi commented Jan 24, 2014

VinInn commented Jan 24, 2014

VinInn commented Jan 24, 2014

anton-a commented Jan 26, 2014

cmsbuild commented Jan 27, 2014

apfeiffer1 commented Jan 28, 2014

anton-a commented Jan 31, 2014

ktf commented Jan 31, 2014

anton-a commented Feb 3, 2014

cmsbuild commented Feb 4, 2014

anton-a commented Feb 4, 2014

cmsbuild commented Feb 4, 2014

cmsbuild commented Feb 4, 2014

davidlange6 commented Feb 4, 2014

ktf commented Feb 5, 2014