Skip to content

Moving MCLabels from digits to global stack#2133

Merged
sawenzel merged 7 commits into
AliceO2Group:devfrom
peressounko:dev
Aug 2, 2019
Merged

Moving MCLabels from digits to global stack#2133
sawenzel merged 7 commits into
AliceO2Group:devfrom
peressounko:dev

Conversation

@peressounko
Copy link
Copy Markdown
Collaborator

No description provided.

@sawenzel
Copy link
Copy Markdown
Collaborator

Weird. There are merge conflicts. Can you please solve them?

@peressounko
Copy link
Copy Markdown
Collaborator Author

Indeed weird. Trying to understand what is going on...

@sawenzel
Copy link
Copy Markdown
Collaborator

There are compilation failures (in ROOT macros). Can you please take a look and fix them?

@sawenzel sawenzel self-requested a review July 2, 2019 20:56
@sawenzel
Copy link
Copy Markdown
Collaborator

Merge conflicts need to be addressed.

Copy link
Copy Markdown
Collaborator

@sawenzel sawenzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline comments

@davidrohr
Copy link
Copy Markdown
Collaborator

@peressounko @sawenzel : Any chance we merge this tomorrow, since this is rather larger, and applying clang-format on the weekend will create merge conflicts. It passes the tests now at least.

<< pos.Y() << ", " << pos.Z() << "), time" << time << ", qdep =" << qdep << std::endl;
mHits->emplace_back(trackID, detID, pos, time, qdep);
// register hit creation with MCStack
static_cast<o2::data::Stack*>(fMC->GetStack())->addHit(GetDetId());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has to stay. it is required to notify someone that a track left a hit in CPV.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sandro, David, I do not plan to touch CPV yet - I will return this lost line.
Presently I implemented filling of MCLabels for PHOS, but I did not try it with DPL framework yet. With old-fashined approach with DigitiserTask I still fighting with problem
[INFO] CREATING BRANCH PHOSDigitMCTruth
[FATAL] No dictionary found for o2::dataformats::MCTruthContainero2::phos::MCLabel
Probably it is better to postpone merging till I will manage to run digitization with DPL.
Or it is easier to merge now as is and continue with new clang formatting? What do you suggest? Thanks, Dmitri

@sawenzel
Copy link
Copy Markdown
Collaborator

sawenzel commented Aug 1, 2019

@davidrohr : Yes we can merge it before the weekend

@davidrohr
Copy link
Copy Markdown
Collaborator

I'll leave it up to you, but if it compiles and doesn't break anything, we can as well merge tomorrow. Then you can continue in a new PR.

@peressounko
Copy link
Copy Markdown
Collaborator Author

OK, then if you agree, lets merge as is. Dmitri

@davidrohr
Copy link
Copy Markdown
Collaborator

ok, @sawenzel : when you have no comments any more, can you merge it?


// Register output containers
mgr->RegisterAny("PHOSDigit", mDigitsArray, kTRUE);
mgr->RegisterAny("PHOSDigitMCTruth", mLabels, kTRUE);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As communicated privately, this IO mechanism is deprecated since we do no longer use the FairTask mechanism.
Rather IO is typically performed by a dedicated DPL writer process. We can adjust this in another round.

@sawenzel
Copy link
Copy Markdown
Collaborator

sawenzel commented Aug 2, 2019

@peressounko : Please don't push more commits to this PR before the weekend. The CI needs quite some time to run and it could delay merging.

#pragma link C++ class o2::phos::Detector+;
#pragma link C++ class o2::phos::GeometryParams+;
#pragma link C++ class o2::base::DetImpl<o2::phos::Detector>+;
#pragma link C++ class o2::phos::MCLabel+;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just adding the label is not enough; you also need to add the specialization ...MCLabelContainer<o2::phos::MCLabel>+ since that is the actual type written to disc.

@sawenzel sawenzel merged commit e94538d into AliceO2Group:dev Aug 2, 2019
knopers8 pushed a commit to knopers8/AliceO2 that referenced this pull request Oct 23, 2019
* Moving MCLabels from digits to global stack

* Adding MCLabel branch to tree
carlos-soncco pushed a commit to carlos-soncco/AliceO2 that referenced this pull request Oct 28, 2019
* Moving MCLabels from digits to global stack

* Adding MCLabel branch to tree
EmilGorm pushed a commit to EmilGorm/AliceO2 that referenced this pull request Apr 15, 2023
Co-authored-by: Dmitri Peresunko <Dmitri.Peresunko@cern.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants