Propagate MC information through ClusterFinder by KlewinS · Pull Request #676 · AliceO2Group/AliceO2

KlewinS · 2017-11-16T14:16:52Z

Propagate MC Truth information
removed unnecessary Box-/HwCluster classes
more efficient thread spawning in ClusterFinder (+configurable at runtime)

KlewinS · 2017-11-16T14:17:53Z

@wiechula could you please have a look, there are quite a lot of changes...

alibuild · 2017-11-16T14:36:03Z

Error while checking build/O2/o2 for 6abacccbbb81584900cf7915332e0cdaa1afe94a:

sw/BUILD/O2-latest/log
  4/130 Test  #27: /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/macro/RawClusterFinder.C ......***Failed    8.83 sec

In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:179:29: error: no matching constructor for initialization of 'o2::TPC::HwClusterer'
    HwClusterer *hwCl = new HwClusterer(&arrCluster, 0, 3);
                            ^           ~~~~~~~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/HwClusterer.h:55:5: note: candidate constructor not viable: no known conversion from 'int' to 'std::unique_ptr<MCLabelContainer> &' (aka 'unique_ptr<MCTruthContainer<o2::MCCompLabel> > &') for 2nd argument
--
class HwClusterer : public Clusterer {
      ^
In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:181:29: error: no viable conversion from 'CalDet<float> *' to 'std::shared_ptr<CalDet<float> >'
    hwCl->setPedestalObject(pedestal);
                            ^~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/GCC-Toolchain/v6.2.0-alice1-1/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.2.0/../../../../include/c++/6.2.0/bits/shared_ptr.h:107:7: note: candidate constructor not viable: no known conversion from 'CalDet<float> *' to 'const std::shared_ptr<o2::TPC::CalDet<float> > &' for 1st argument
--
    void setPedestalObject(std::shared_ptr<CalDet<float>> pedestalObject) { mPedestalObject = pedestalObject; };
                                                          ^
In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:194:7: error: no member named 'Init' in 'o2::TPC::Clusterer'
  cl->Init();
  ~~  ^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:209:111: warning: format specifies type 'int' but the argument has underlying type 'char' [-Wformat]
    printf("========| Event %4zu %d %d %d |========\n", converter.getPresentEventNumber(), events, maxEvents, status);
                                       ~~                                                                     ^~~~~~
                                       %hhd
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:215:9: error: no matching member function for call to 'Process'
    cl->Process(arr);
    ~~~~^~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Clusterer.h:57:18: note: candidate function not viable: requires 3 arguments, but 1 was provided
--

In file included from input_line_8:1:
In file included from /mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/runRawClusterFinder.C:11:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:179:29: error: no matching constructor for initialization of 'o2::TPC::HwClusterer'
    HwClusterer *hwCl = new HwClusterer(&arrCluster, 0, 3);
                            ^           ~~~~~~~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/HwClusterer.h:55:5: note: candidate constructor not viable: no known conversion from 'int' to 'std::unique_ptr<MCLabelContainer> &' (aka 'unique_ptr<MCTruthContainer<o2::MCCompLabel> > &') for 2nd argument
--
      ^
In file included from input_line_8:1:
In file included from /mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/runRawClusterFinder.C:11:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:181:29: error: no viable conversion from 'CalDet<float> *' to 'std::shared_ptr<CalDet<float> >'
    hwCl->setPedestalObject(pedestal);
                            ^~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/GCC-Toolchain/v6.2.0-alice1-1/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.2.0/../../../../include/c++/6.2.0/bits/shared_ptr.h:107:7: note: candidate constructor not viable: no known conversion from 'CalDet<float> *' to 'const std::shared_ptr<o2::TPC::CalDet<float> > &' for 1st argument
--
                                                          ^
In file included from input_line_8:1:

Full log here.

alibuild · 2017-11-16T14:37:03Z

Error while checking build/O2/o2-dev-fairroot for 6abacccbbb81584900cf7915332e0cdaa1afe94a:

sw/BUILD/O2-latest/log
  1/130 Test  #34: /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/macro/testClustererData.C .....***Failed    7.13 sec

In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/testClustererData.C:51:15: error: no matching constructor for initialization of 'o2::TPC::HwClusterer'
  HwClusterer cl(&arrCluster);
              ^  ~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/HwClusterer.h:39:7: note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'std::vector<o2::TPC::Cluster> *' to 'const o2::TPC::HwClusterer' for 1st argument
--
  2/130 Test  #27: /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/macro/RawClusterFinder.C ......***Failed   10.74 sec

In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:179:29: error: no matching constructor for initialization of 'o2::TPC::HwClusterer'
    HwClusterer *hwCl = new HwClusterer(&arrCluster, 0, 3);
                            ^           ~~~~~~~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/HwClusterer.h:55:5: note: candidate constructor not viable: no known conversion from 'int' to 'std::unique_ptr<MCLabelContainer> &' (aka 'unique_ptr<MCTruthContainer<o2::MCCompLabel> > &') for 2nd argument
--
class HwClusterer : public Clusterer {
      ^
In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:181:29: error: no viable conversion from 'CalDet<float> *' to 'std::shared_ptr<CalDet<float> >'
    hwCl->setPedestalObject(pedestal);
                            ^~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/GCC-Toolchain/v6.2.0-alice1-1/bin/../lib/gcc/x86_64-unknown-linux-gnu/6.2.0/../../../../include/c++/6.2.0/bits/shared_ptr.h:107:7: note: candidate constructor not viable: no known conversion from 'CalDet<float> *' to 'const std::shared_ptr<o2::TPC::CalDet<float> > &' for 1st argument
--
    void setPedestalObject(std::shared_ptr<CalDet<float>> pedestalObject) { mPedestalObject = pedestalObject; };
                                                          ^
In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:194:7: error: no member named 'Init' in 'o2::TPC::Clusterer'
  cl->Init();
  ~~  ^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:209:111: warning: format specifies type 'int' but the argument has underlying type 'char' [-Wformat]
    printf("========| Event %4zu %d %d %d |========\n", converter.getPresentEventNumber(), events, maxEvents, status);
                                       ~~                                                                     ^~~~~~
                                       %hhd
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:215:9: error: no matching member function for call to 'Process'
    cl->Process(arr);
    ~~~~^~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Clusterer.h:57:18: note: candidate function not viable: requires 3 arguments, but 1 was provided
--

In file included from input_line_8:1:
In file included from /mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/runRawClusterFinder.C:11:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/reconstruction/macro/RawClusterFinder.C:179:29: error: no matching constructor for initialization of 'o2::TPC::HwClusterer'
    HwClusterer *hwCl = new HwClusterer(&arrCluster, 0, 3);
                            ^           ~~~~~~~~~~~~~~~~~
/mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/HwClusterer.h:55:5: note: candidate constructor not viable: no known conversion from 'int' to 'std::unique_ptr<MCLabelContainer> &' (aka 'unique_ptr<MCTruthContainer<o2::MCCompLabel> > &') for 2nd argument
--
      ^
In file included from input_line_8:1:

Full log here.

alibuild · 2017-11-16T14:43:54Z

Error while checking build/o2/macos for 6abacccbbb81584900cf7915332e0cdaa1afe94a:

sw/BUILD/FairRoot-latest/log
[ 51%] Building CXX object base/CMakeFiles/Base.dir/field/FairField.cxx.o
[ 51%] Linking CXX shared library ../lib/libFairMQ.dylib
ld: file not found: libboost_thread.dylib for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libFairMQ.dylib] Error 1
make[1]: *** [fairmq/CMakeFiles/FairMQ.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

Full log here.

wiechula

Somehow I don't find the point where the MC truth for the clusters is actually calculated. It looks as if mcDigitTruth and mClusterMcLabelArray (in the HwClusterer) are not used.

Is this only a preparation?

KlewinS · 2017-11-16T14:54:28Z

HwClusterer.cxx, line 443 to 447

the MC Truth information of the digits is kept in this class and not propagated to the actual CF, but the digit-Index and event number of the digit. With those, the used digits can be identified again and the MC Truth information can be collected and combined at this point.

wiechula · 2017-11-16T15:19:10Z

I see, the code was folded, so I didn't spot it in the first palce.
In the digitisation ( DigitPad::fillOutputContainer) the labels are weighted by how often they occur and then sorted for their occurrence (most probable id first). Is something similar done for the clusers? I don't find it.

Did you check the results of the cluster finder before and after the changes using the same input digits?

KlewinS · 2017-11-16T15:27:26Z

No, I didn't implement a sorting because I couldn't see why this should make a difference. In the end it would only change in which order the labels are inserted . Shouldn't the container take care of something like this? There the data index is used to lookup the header, wich is then incremented. Why makes the order here any difference?

I didn't do a detailed analysis of the resulting found clusters, but the histograms in the root file look reasonable.

KlewinS · 2017-11-16T15:35:05Z

Maybe I just don't understand how this container really works. I will add a sorting stage.

wiechula · 2017-11-16T16:21:32Z

In the digitisation there is an intermediate stage where per digit we count how often a label occurs. Then, before filling the MC truth output container the labels are sorted according to their occurrence. This is important, because otherwise you don't know which is the most probable label. Like this we know the first label ist the most probable.
There is no weight in the label itself. We might discuss if we need this.

KlewinS · 2017-11-17T13:01:54Z

@wiechula sorting should now be there

alibuild · 2017-11-17T13:10:06Z

Error while checking build/O2/o2-dev-fairroot for 73f9380721034aa3dff7dcaa4e7d994f3f6cf654:

sw/BUILD/O2-latest/log
  7/130 Test  #38: /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C ....***Failed   13.95 sec

In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:159:24: error: no member named 'HwCluster' in namespace 'o2::TPC'
  std::vector<o2::TPC::HwCluster>  *clustersArray = nullptr;
              ~~~~~~~~~^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:159:37: error: C++ requires a type specifier for all declarations
  std::vector<o2::TPC::HwCluster>  *clustersArray = nullptr;
                                    ^
Warning in <TInterpreter::LoadLibraryMap>: Problems in /mnt/mesos/sandbox/sandbox/sw/BUILD/eaf8e68bbb45a73cbd714c74ab23104155986086/O2/lib/libTPCReconstruction.rootmap declaring 'namespace std {  }namespace o2 { namespace TPC {  } }' were encountered.
--

#undef  _BACKWARD_BACKWARD_WARNING_H

/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:242:15: error: use of undeclared identifier 'TrackTPC'
  std::vector<TrackTPC> *arrTracks = 0;
              ^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:242:26: error: C++ requires a type specifier for all declarations
  std::vector<TrackTPC> *arrTracks = 0;
                         ^
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairTimeStamp.h:11:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData_Interface.h:13:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairLink.h:104:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, const FairLink& link) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairLink.h:104:26: note: previous definition is here
--
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairTimeStamp.h:11:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData_Interface.h:14:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData.h:95:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, const FairMultiLinkedData& data) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData.h:95:26: note: previous definition is here
--
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairTimeStamp.h:11:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData_Interface.h:73:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, FairMultiLinkedData_Interface& data) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairMultiLinkedData_Interface.h:73:26: note: previous definition is here
--
                         ^
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/0_O2_DEV_FAIRROOT/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/dev-1/include/FairTimeStamp.h:66:26: error: redefinition of 'operator<<'

Full log here.

alibuild · 2017-11-17T13:12:44Z

Error while checking build/o2/macos for 73f9380721034aa3dff7dcaa4e7d994f3f6cf654:

Full log here.

alibuild · 2017-11-17T13:13:51Z

Error while checking build/O2/o2 for 73f9380721034aa3dff7dcaa4e7d994f3f6cf654:

sw/BUILD/O2-latest/log
 11/130 Test  #38: /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C ....***Failed   13.34 sec

In file included from input_line_8:1:
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:159:24: error: no member named 'HwCluster' in namespace 'o2::TPC'
  std::vector<o2::TPC::HwCluster>  *clustersArray = nullptr;
              ~~~~~~~~~^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:159:37: error: C++ requires a type specifier for all declarations
  std::vector<o2::TPC::HwCluster>  *clustersArray = nullptr;
                                    ^
Warning in <TInterpreter::LoadLibraryMap>: Problems in /mnt/mesos/sandbox/sandbox/sw/BUILD/5e8e57f3af13919aadf13392ef7bfe5bae1fd086/O2/lib/libTPCReconstruction.rootmap declaring 'namespace std {  }namespace o2 { namespace TPC {  } }' were encountered.
--

#undef  _BACKWARD_BACKWARD_WARNING_H

/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:242:15: error: use of undeclared identifier 'TrackTPC'
  std::vector<TrackTPC> *arrTracks = 0;
              ^
/mnt/mesos/sandbox/sandbox/O2/Detectors/TPC/simulation/macro/testHitsDigitsClusters.C:242:26: error: C++ requires a type specifier for all declarations
  std::vector<TrackTPC> *arrTracks = 0;
                         ^
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairTimeStamp.h:11:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData_Interface.h:13:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairLink.h:104:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, const FairLink& link) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairLink.h:104:26: note: previous definition is here
--
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairTimeStamp.h:11:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData_Interface.h:14:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData.h:95:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, const FairMultiLinkedData& data) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData.h:95:26: note: previous definition is here
--
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
In file included from /mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairTimeStamp.h:11:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData_Interface.h:73:26: error: redefinition of 'operator<<'
    friend std::ostream& operator<< (std::ostream& out, FairMultiLinkedData_Interface& data) {
                         ^
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairMultiLinkedData_Interface.h:73:26: note: previous definition is here
--
                         ^
In file included from G__TPCReconstructionDict dictionary payload:94:
In file included from /mnt/mesos/sandbox/sandbox/sw/SOURCES/O2/dev/0/Detectors/TPC/reconstruction/include/TPCReconstruction/Cluster.h:20:
/mnt/mesos/sandbox/sandbox/sw/slc7_x86-64/FairRoot/alice-dev-20171027-4/include/FairTimeStamp.h:66:26: error: redefinition of 'operator<<'

Full log here.

alibuild · 2017-11-17T13:20:07Z

Error while checking build/o2/macos for 44ce376d630392c35ee3cddbd133d4a3c4371dd3:

sw/BUILD/FairRoot-latest/log
[ 47%] Building CXX object base/CMakeFiles/Base.dir/event/FairFileInfo.cxx.o
[ 47%] Linking CXX shared library ../lib/libFairMQ.dylib
ld: file not found: libboost_thread.dylib for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libFairMQ.dylib] Error 1
make[1]: *** [fairmq/CMakeFiles/FairMQ.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

Full log here.

shahor02 · 2017-11-17T14:12:09Z

@KlewinS it will be much faster to use labelCount[l]++ instead of try/catch

indeed, using labelCount[l]++ faster

i'd say "compacter" - nothing about std::map is fast :)

speaking of which: since you use it in a tight loop - did you profile the code? maybe using a more performant container would help if you have a hotspot here - in many cases a simple sorted vector (or array) performs better with a binary lookup.

@mkrzewic : don't exclude that sorted array will be faster, but the labelCount.at(l)++ is indeed faster than try/catch: e.g. for (int j=10;j--;) {labelCount[gRandom->Integer(10)]++; } is faster that the try/catch equivalent by factor >30.

I don't think, this will be the bottleneck and it's done only once per event. But I will keep in mind that this could be improved if we have to speed up this part.

@shahor02 factor 30 seems a bit much, but indeed, the "proper way" with the direct increment is faster by ~2X if we have mostly duplicated entries to ~7X is we always throw for all unique entries (with keys that are as large as here). (llvm 7.3, -O2, on my laptop).

KlewinS · 2017-11-20T08:13:50Z

@sawenzel can we have a copy-constructor for the MCTruthContainer? Currently I have to copy it here element by element.

@KlewinS : will take a look

matthiasrichter

I have a few minor comments and questions for my understanding.

matthiasrichter · 2017-11-20T09:58:56Z

better set these parameters as defaults in the extended constructor and forbid that one (Clusterer() = delete;)

matthiasrichter · 2017-11-20T10:06:13Z

"pointer to HW Clusterer"

matthiasrichter · 2017-11-20T10:34:51Z

just out of curiosity, is the clusterization running on simulated and HW clusters in the same process so that it makes sense to carry the two types of arrays? Or is it more a kind of a specialization since the task is running on either the one or the other?

There is still this BoxClusterer, which could in principle also be used. Although I think the BoxClusterer doesn't work anymore.

matthiasrichter · 2017-11-20T10:39:19Z

is this some kind of FIFO, or what does the Last mean? If I understand correctly, this is supposed to keep a set of containers for multiple events, correct? I don't see it used in the code though. How does this relate to the above defined mClusterMcLabelArray?

Yes, it is a kind of FIFO and is used only in HwClusterer.cxx line 460.

It stores the MC Truth information of the last event(s) because the actual ClusterFinder doesn't work only on digits of the "current" event. The digits there are kept also across event-boundaries to enable the continuous simulation.

During this, I also found, that there are some events in the simulation consisting of only very few timebins (even only a single timebin). That is, why it is not enough to store only the information from the "last" event, but from several previous events.

Is anything missing to accept this pull request?

fix gsl include directory WIP: Cluster MCLabels

WIP: Cluster MCLabels, cleaning up WIP: Cluster MCLabels, fix buffering of MC labels of previous events

WIP: Cluster MCLabels, cleaning up continued finished Cluster MCLabels + cleaning up

also wrong include paths, introduced by to moving Cluster related classes into reconstruction

sawenzel · 2017-11-22T07:28:38Z


+    // Register MC Truth output container
+    mHwClustersMCTruthArray = std::make_unique<MCLabelContainer>();
+    mgr->Register("TPCClusterHWMCTruth", "TPC", mHwClustersMCTruthArray.get(), kTRUE);


a tiny thing here: If we register the branch like this, we have the well known problem that the TBrowser cannot draw from the branch (see ROOT issue: https://sft.its.cern.ch/jira/browse/ROOT-8993)

According to the ROOT team, the correct way to do it (in this case) is to add a trailing dot to the branch name like this
TPCClusterHWMCTruth.; I verified that this work but it needs adjustment also in the macro reading the clusters.

Shall we do it for the sake of usabibility? I can add this fix and merge it quickly.

Yes, this sounds good. For me it is fine if you do this.
Please go ahead

sawenzel · 2017-11-22T07:30:10Z

@KlewinS , @wiechula: We could merge it. Just waiting for short feedback to my latest question on the branch naming.

* Add whitespace formatting check * Mark incorrect copyright notices in GitHub UI

wiechula reviewed Nov 16, 2017

View reviewed changes

KlewinS force-pushed the dev branch from 73f9380 to 44ce376 Compare November 17, 2017 13:13

shahor02 reviewed Nov 17, 2017

View reviewed changes

KlewinS force-pushed the dev branch 2 times, most recently from d486d2e to 9a682cf Compare November 17, 2017 14:53

KlewinS commented Nov 20, 2017

View reviewed changes

matthiasrichter reviewed Nov 20, 2017

View reviewed changes

Sebastian Klewin and others added 9 commits November 20, 2017 15:39

add Cluster MCLabels

30513e5

fix gsl include directory WIP: Cluster MCLabels

replaced BoxCluster with Cluster, additional information were never used

c66033e

replaced HwCluster with Cluster, additional information were never used

ed7fd88

removed Box-/HwCluster files

16632d8

continued add Cluster MCLabels

4d8f2d2

WIP: Cluster MCLabels, cleaning up WIP: Cluster MCLabels, fix buffering of MC labels of previous events

more efficient thread spawning + num threads configurable at runtime

fddec3a

continued Cluster MCLabels + cleaning up

0637d57

WIP: Cluster MCLabels, cleaning up continued finished Cluster MCLabels + cleaning up

fixed some compiler warnings, comparison signed/unsigned

93cb1b1

add MC label sorting

444caa1

Sebastian Klewin added 3 commits November 20, 2017 15:39

fix macro compilation error

f3a0f20

also wrong include paths, introduced by to moving Cluster related classes into reconstruction

removed try/catch construct

e9b7360

implemented Matthias' comments + fix padMean

92d2d3f

KlewinS force-pushed the dev branch from 2b97c2f to 92d2d3f Compare November 20, 2017 14:41

sawenzel reviewed Nov 22, 2017

View reviewed changes

sawenzel merged commit 7aed09c into AliceO2Group:dev Nov 22, 2017

arvindkhuntia pushed a commit to arvindkhuntia/AliceO2 that referenced this pull request Jun 6, 2022

Improve code formatting checks (AliceO2Group#676)

0fa9d79

* Add whitespace formatting check * Mark incorrect copyright notices in GitHub UI

Conversation

KlewinS commented Nov 16, 2017

Uh oh!

KlewinS commented Nov 16, 2017

Uh oh!

alibuild commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alibuild commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alibuild commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wiechula left a comment

Choose a reason for hiding this comment

Uh oh!

KlewinS commented Nov 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wiechula commented Nov 16, 2017

Uh oh!

KlewinS commented Nov 16, 2017

Uh oh!

KlewinS commented Nov 16, 2017

Uh oh!

wiechula commented Nov 16, 2017

Uh oh!

KlewinS commented Nov 17, 2017

Uh oh!

alibuild commented Nov 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alibuild commented Nov 17, 2017

Uh oh!

alibuild commented Nov 17, 2017

Uh oh!

alibuild commented Nov 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthiasrichter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibuild commented Nov 16, 2017 •

edited

Loading

alibuild commented Nov 16, 2017 •

edited

Loading

alibuild commented Nov 16, 2017 •

edited

Loading

KlewinS commented Nov 16, 2017 •

edited

Loading

alibuild commented Nov 17, 2017 •

edited

Loading