ZDC-like Digits and RecPoints structure

[AllaMaevskaya]

No description provided.

[sawenzel]

including iostream in headers should be avoided. Why do you need it?

[sawenzel]

please remove comment

[sawenzel]

please remove comment

[sawenzel]

please remove comment

[sawenzel]

could you add a sentence of what is "Temp" here? Is it some intermediate format?

[sawenzel]

please remove comment if this is no longer needed

[sawenzel]

@MichaelLettrich : MacOS tells:

-- The Fortran compiler identification is unknown
CMake Error at CMakeLists.txt:4 (Enable_Language):
  No CMAKE_Fortran_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "FC" or the CMake cache entry CMAKE_Fortran_COMPILER to the full
  path to the compiler, or to the compiler name if it is in the PATH.

Is this a new problem?

[sawenzel]

@AllaMaevskaya : Thanks for this comprehensive PR. Could you just please provide a description of what it achieves? Is this implementing pileup/embedding?

[shahor02]

@AllaMaevskaya Could you please remind the range of these data members? Do you really need 4 ints?

[AllaMaevskaya]

DetID - 0-208
CFDTime is 12 bits of RawData
QTCAmpl - 13 bit
ChainQTC can be 1 or 0

You suggest to use RawEventData uint64_t word to keep it in Digits?

[shahor02]

You can simply use

uint8_t ChanID
int8_t ChainQTC
int16_t CFDTime
int16_t QTCAmpl

will take the same 8 bytes (instead of current 16) and w/o bit operations.

[AllaMaevskaya]

done, thank you

[shahor02]

Do you really need doubles here (and again, what is the range of int members?) Anyway, if the types cannot be changed, please move ChainQTC right after the ChId, this will save 8 B in the sizeof(ChannelDataFloat)

[shahor02]

could you please define in the Geometry the inverse of MV_2_Nchannels and multiply by it?

[shahor02]

idem, if possible, multiply by precalculated inverse instead of dividing.

[shahor02]

here too, if possible, multiply by precalculated inverse instead of dividing.

[shahor02]

here too

[shahor02]

why are you getting a vector here and not a span?

[shahor02]

This question was not answered

[shahor02]

@AllaMaevskaya , did you miss this?

[AllaMaevskaya]

Sorry, I really misread question.
Digits are structure with event-wise data like BC, orbit and triggers and span of ChannelData, similar to ZDC as you requested. This Digits are collected in vector event-by-event. What is wrong?

[shahor02]

When you do pc.inputs().get<const std::vector... you create a local copy of the framework-owned data. This is a waste of the memory and CPU resources. Instead on can use span, which is essentially a pointer on these data. But using the span requires that elements of this span (Digits in your case) are copiable (do not contain pointers, vectors etc). So, the whole point of these ZDC-like structures was to make T0 objects copiable and use spans instead of the vectors. Just change auto digits = pc.inputs().get<const std::vector<o2::ft0::Digit>>("digits"); to
auto digits = pc.inputs().get<gsl::span<o2::ft0::Digit>>("digits");

[AllaMaevskaya]

Done

[shahor02]

@sawenzel the bulk of this PR is getting rid of the std::vectors in the digits and recpoints (e.g. to avoid root seralization at messaging).

[AllaMaevskaya]

Here is detail description of what is in PR:

• embedding is implemented, pile-up not;
• TCM (trigger module) data v0 (v1 is comming) added in raw data;
• FT0 Digits and RecPoints are vectors with each element is event (BC/Orbit) related information as triggers and reconstructed collision time and vertex, and time and amplitude signals in fired channels written as a span.

[AllaMaevskaya]

Sorry , I found bug. Please wait until tomorrow.
Can you tell me: ROOT can not treat structure union correctly, yes?

[shahor02]

@AllaMaevskaya by initializing the uint8_t wih -1 you are assigning to 256. Is this what you want?

[AllaMaevskaya]

no, I fixed it now

[shahor02]

But you wrote before that the chanID goes from 0 to 208, with in8_t you cannot cover this range. If you want to initialize with the value which is outside of the allowed range, use uin8_t and init with 0xff

[shahor02]

@AllaMaevskaya could you please apply clang-format ?

[AllaMaevskaya]

Hello

in the morning all test were green, now 2 of them are red and logs are not visible. What should I do?

[shahor02]

The test servers are not available due to the problems in IT. Once it gets green again, will merge it. Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: AllaMaevskaya <notifications@github.com> Sent: Thursday, February 20, 2020 2:33:34 PM To: AliceO2Group/AliceO2 <AliceO2@noreply.github.com> Cc: Ruben Shahoyan <shahor02@gmail.com>; Comment <comment@noreply.github.com> Subject: Re: [AliceO2Group/AliceO2] ZDC-like Digits and RecPoints structure (#2926) Hello in the morning all test were green, now 2 of them are red and logs are not visible. What should I do? — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#2926?email_source=notifications&email_token=ABYKIDM2SDFKSR462SCXFGTRD2BC5A5CNFSM4KSDUHWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMN42ZY#issuecomment-589024615>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABYKIDL47W6MVCW2J7UIWN3RD2BC5ANCNFSM4KSDUHWA>.

[ktf]

@shahor02 Everything is back to green here. Squash and merge?