[MFT] Move static arrays to be initialized before the processing starts by robincaron13 · Pull Request #10665 · AliceO2Group/AliceO2

robincaron13 · Feb 1, 2023

To follow PR#10654, here the arrays are kept as static member and are moved to the tracker class to be initialized before the processing starts (not sure if it was what you were thinking @shahor02)

shahor02

Hi @robincaron13
Thanks for taking care, but this not what I meant: moving the static array from the base class to the derived one will change nothing: it will still be allocated w/o the creation of the actual object, i.e. in any process which links to this library.
Instead, I would define in the TrackerConfig.h

using BinContainer = std::array<std::array<std::array<std::vector<Int_t>, constants::index_table::MaxRPhiBins>, (constants::mft::LayersNumber - 1)>, (constants::mft::LayersNumber - 1)>;

static std_unique_ptr<BinContainer> mBins;
static std_unique_ptr<BinContainer> mBinsS;
static initBinContainers() { // implementation can go to cxx file
  if (!mBins) {
    mBins = std::make_unique<BinContainer>();
    initBins(mBins.get()); // init bins with static method. If not possible to do using static method, then make this method non-static and lock the code by mutex.
   //
  }
  // same for mBinsS
}

In the TrackerSpec call this initBinContainers before instantiating the trackers (e.g. in its init method (or in the beginning of updateTimeDependentParams) if this method can be static.
Otherwise, put the non-static method (protected by mutex) in the Tracker constructed.
Same for destroying arrays via mBins.reset() : do this either in the Tracker destructor of the protecting the call with mutex or in the TrackerSpec destructor.

For the usage of the mutex see e.g. how the

AliceO2/Detectors/Base/include/DetectorsBase/GeometryManager.h

Line 140 in 816da25

static std::mutex sTGMutex;

is used in

AliceO2/Detectors/Base/src/GeometryManager.cxx

Lines 37 to 48 in 816da25

    
           std::mutex GeometryManager::sTGMutex; 
        
           //______________________________________________________________________ 
        
           Bool_t GeometryManager::getOriginalMatrix(const char* symname, TGeoHMatrix& m) 
        
           { 
        
             m.Clear(); 
        
             if (!gGeoManager || !gGeoManager->IsClosed()) { 
        
               LOG(error) << "No active geometry or geometry not yet closed!"; 
        
               ; 
        
               return kFALSE; 
        
             } 
        
             std::lock_guard<std::mutex> guard(sTGMutex);

and below.

robincaron13 · Feb 2, 2023

Hi @shahor02,

Thanks a lot for your answer and your proposal.
Currently I'm making the changes but I have a question regarding the initBins. What should contain this function exactly? Because from my understanding an arrays is intialized at the moment we define its size but here it's now about std::unique_ptr, also I tried different way like for example using std::unique_ptr<BinContainer>(new BinContainer) and no success... Sorry I'm not an expert in the use of some functionalities in c++.

shahor02 · Feb 2, 2023

I have a question regarding the initBins. What should contain this function exactly?
Hi @robincaron13
The initialization of the uniqe_ptr will just allocate the memory but not assign the values you need.
The initBins should contain what

AliceO2/Detectors/ITSMFT/MFT/tracking/src/Tracker.cxx

Lines 198 to 214 in 50d8566

mBinsS[layer1][layer2 - 1][binIndex1].emplace_back(binIndex2S);

}

}

int binwPhi = mPhiBinWin[layer2];

int binhwPhi = binwPhi / 2;

for (Int_t binR = rBinMin; binR <= rBinMax; ++binR) {

for (Int_t iPhi = 0; iPhi < binwPhi; ++iPhi) {

int binPhi = binPhi_proj + (iPhi - binhwPhi);

if (binPhi < 0) {

continue;

}

binIndex2 = getBinIndex(binR, binPhi);

mBins[layer1][layer2 - 1][binIndex1].emplace_back(binIndex2);

does: assignment of the values.
I did not check if

AliceO2/Detectors/ITSMFT/MFT/tracking/src/Tracker.cxx

Line 109 in 50d8566

void Tracker<T>::initializeFinder()

does anything else but setting mBins(S) values, if this function should be called only once before the processing starts, then this is actually the "initBins" I meant.

robincaron13 · Feb 2, 2023

Hi @shahor02,

Thanks for your answer. Some modifications are pushed according to your comments.
For the moment I still kept a dummy static initBins in the TrackerConfig.cxx to do nothing, also basically the initBinConntainer is just doing mBins = std::make_unique<BinContainer>(); and it should be called once in the init of the TrackerSpec.cxx. The mBins.reset() is called through the new destructor called in TrackerSpec.cxx. The mutex is also added to the intializeFinder funnction where the assignment of the mBins values are perfomed.
Let me know if it goes in the good direction, or there are still modifications to do.

shahor02

Thanks @robincaron13
In general ok, but please see some comments below.

Detectors/ITSMFT/MFT/tracking/include/MFTTracking/TrackerConfig.h

Detectors/ITSMFT/MFT/tracking/src/Tracker.cxx

Detectors/ITSMFT/MFT/tracking/src/TrackerConfig.cxx

Detectors/ITSMFT/MFT/workflow/src/TrackerSpec.cxx

robincaron13 · Feb 6, 2023

Hi @shahor02
Let me know if the last changes added were fine with you. Thanks.

shahor02

thanks, looks good

robincaron13 added 2 commits February 1, 2023 22:18

move mBins static arrays to tracker initialization

52bd7bb

move also arrays with smaller sizes

0aeaca9

robincaron13 requested review from bovulpes, iouribelikov, mconcas and rpezzi as code owners February 1, 2023 22:29

shahor02 reviewed Feb 1, 2023

View reviewed changes

add std::unique_ptr<BinContainer> mBins and associated init functions

c631cb9

shahor02 requested changes Feb 2, 2023

View reviewed changes

add the requested changes: instantation of arrays and tracker destructor

f796beb

robincaron13 requested a review from shahor02 February 3, 2023 09:55

remove include not needed

fcc42c7

shahor02 approved these changes Feb 6, 2023

View reviewed changes

shahor02 merged commit 0b380ab into AliceO2Group:dev Feb 6, 2023

robincaron13 mentioned this pull request Feb 6, 2023

[MFT] Add new default settings for MFT track finder AliceO2Group/O2DPG#852

Merged

This was referenced Feb 16, 2023

gpu dynamic memory mconcas/AliceO2#49

Closed

ITS-SV: Use dynamic TimeFrame splitting for GPU vertexer mconcas/AliceO2#50

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[MFT] Move static arrays to be initialized before the processing starts#10665

[MFT] Move static arrays to be initialized before the processing starts#10665
shahor02 merged 5 commits intoAliceO2Group:devAliceO2Group/AliceO2:devfrom
robincaron13:mft-fix-trackingrobincaron13/AliceO2:mft-fix-trackingCopy head branch name to clipboard

robincaron13 commented Feb 1, 2023

Uh oh!

shahor02 left a comment

Uh oh!

robincaron13 commented Feb 2, 2023

Uh oh!

shahor02 commented Feb 2, 2023

Uh oh!

robincaron13 commented Feb 2, 2023 •

edited

Loading

Uh oh!

shahor02 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robincaron13 commented Feb 6, 2023

Uh oh!

shahor02 left a comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

	std::mutex GeometryManager::sTGMutex;

	//______________________________________________________________________
	Bool_t GeometryManager::getOriginalMatrix(const char* symname, TGeoHMatrix& m)
	{
	m.Clear();
	if (!gGeoManager \|\| !gGeoManager->IsClosed()) {
	LOG(error) << "No active geometry or geometry not yet closed!";
	;
	return kFALSE;
	}
	std::lock_guard<std::mutex> guard(sTGMutex);

Search code, repositories, users, issues, pull requests...

Comments

Conversation

robincaron13 commented Feb 1, 2023

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

robincaron13 commented Feb 2, 2023

Uh oh!

shahor02 commented Feb 2, 2023

Uh oh!

robincaron13 commented Feb 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robincaron13 commented Feb 6, 2023

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

robincaron13 commented Feb 2, 2023 •

edited

Loading