[EMCAL-610, EMCAL-659, EMCAL-661] EMCAL digit to raw converter stand alone application by mfasDa · Pull Request #4045 · AliceO2Group/AliceO2

mfasDa · 2020-07-27T14:51:43Z

Change output handling in RawWriter from RawPageHandler to RawFileWriter and make use of its methods to create timeframes and superpages
Add stand alone application which reads the digit file, passes the digit and trigger branch to the RawWriter and launches the raw encoding.

mfasDa · 2020-07-27T15:21:49Z

Pull request will be extended by a stand-alone application converting digit files to raw data (separate ticket ID).

mfasDa · 2020-08-03T11:18:06Z

@shahor02 @sawenzel The first IR should be the same for the digitizer and for the raw writer, how do we propagate it in the best way.

Currently for EMCAL the digits vector contains all digits in the timeframe, separated by triggers using a second vector with a range reference marking the digits belonging to the same readout window (trigger). This is done in order to implement the handling of pileup in the EMCAL. Live/Dead times are handled there as well. In the digitizer we use the collision context to get the IR, set the detector live for the first trigger, accumulate digits from collisions within the 1.5 ms readout window digitised using the EMCAL time response, and set the detector busy afterwards. Therefore even though for the moment the first collision opens the EMCAL readout window we cannot blindly use the first IR in the timeframe as in the future it might not be the first collision.

shahor02 · 2020-08-03T11:31:52Z

Hi @mfasDa

I have not really understood the question. The 1st IR of digitization can be anything, it is where the collision happened (currently the sampling starts for orbit/bc = 1/0, can be overridden by --firstOrbit, --firstBC options to digitizer), the 1st IR of raw data is the start of the 1st TF, i.e. detectors data packaging (defined by the configurable param

AliceO2/Detectors/Raw/include/DetectorsRaw/HBFUtils.h

Lines 126 to 128 in b5b5389

    
           int nHBFPerTF = 1 + 0xff; // number of orbits per BC 
        
           uint16_t bcFirst = 0;     ///< BC of 1st TF 
        
           uint32_t orbitFirst = 0;  ///< orbit of 1st TF

). For consistence, the 1st TF 1st IR must be <= first collision IR.

Cheers,
Ruben

mfasDa · 2020-08-03T11:48:48Z

@shahor02 The problem is that our readout window with 1.5 mus >> bunch spacing with 25 ns. Therefore in the digitization in order to simulate pileup we integrate over as many collisions as we can fit into the readout window (-600 ns due to delays in the cable). Therefore we decided to store the digits not per event - which would be anyhow questionable with pileup - but per timeframe, splitting the digits vector in ranges belonging to the same readout window. Therefore we assume in the digitization that all collisions we loop over belong to the same timeframe, and we push the digits vector once per timeframe. Doing this, we must have the same timing definition in the digitizer and the raw writer. We could simply mark the first IR but it might be that EMCAL will not trigger at the first IR and therefore set a wrong IR as first IR - which should be common for all detectors. Otherwise I would be curious how other detectors do it - in particular the continuous detectors must integrate in the digitization over many events and are busy at the end of timeframe, no?

shahor02 · 2020-08-03T12:03:49Z

@mfasDa: all detectors store their data in this way, ITS readout window can be as large as 20 \mus, it does not matter. On the digitization level we don't have TF definition at all, but every digit is associated with well defined BC (when the trigger or strobe started). It is irrelevant what other (later) BCs will contribute to this digit, at digit->raw conversion it goes to the TF which its opening BC belongs to.

mfasDa · 2020-08-03T12:23:58Z

@shahor02 OK, for us all the digits in the same open readout window have the same IR, the one of the trigger which opened the readout window, and the raw writer makes use of it. Shall we rather push digits in readout windows?

shahor02 · 2020-08-03T12:35:56Z

Is not the digit already covering exactly a readout window? What do you mean pushing them in readout window? In terms of the RawWriter you add a payload for the IR of the beginning of the readout window. Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: Markus Fasel <notifications@github.com> Sent: Monday, August 3, 2020 2:24:13 PM To: AliceO2Group/AliceO2 <AliceO2@noreply.github.com> Cc: Ruben Shahoyan <shahor02@gmail.com>; Mention <mention@noreply.github.com> Subject: Re: [AliceO2Group/AliceO2] [EMCAL-610, EMCAL-659, EMCAL-661] EMCAL digit to raw converter stand alone application (#4045) @shahor02<https://github.com/shahor02> OK, for us all the digits in the same open readout window have the same IR, the one of the trigger which opened the readout window, and the raw writer makes use of it. Shall we rather push digits in readout windows? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#4045 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABYKIDPP5AKDWV2TNE73OG3R62T63ANCNFSM4PI3CWYA>.

shahor02

Hi @mfasDa

Please see a comment below on some needed functionalities of the converter. Apart from these minor details, is this PR ready to be tested/merged?

shahor02 · 2020-08-10T22:45:14Z

Detectors/EMCAL/simulation/src/RawCreator.cxx

Could you please add the following options:

possibility to produce 1 raw file per link (i.e. FEEID), this is preferred mode for replay using readout.exe. See how it is done e.g. for ITS:

AliceO2/Detectors/ITSMFT/ITS/simulation/src/digi2raw.cxx

Line 61 in b0fc846

add_option("file-for,f", bpo::value<std::string>()->default_value("layer"), "single file per: all,layer,cru,link");

parsing configKeyValues options, i.e.

AliceO2/Detectors/ITSMFT/ITS/simulation/src/digi2raw.cxx

Line 65 in b0fc846

add_option("configKeyValues", bpo::value<std::string>()->default_value(""), "comma-separated configKeyValues");

AliceO2/Detectors/ITSMFT/ITS/simulation/src/digi2raw.cxx

Line 85 in b0fc846

o2::conf::ConfigurableParam::updateFromString(vm["configKeyValues"].as<std::string>());

This is necessary for e.g. generation of raw data with custom number of HBFs per TF.

Also, all detectors generate in the output directory a configuration file which can be provided to RawFileReader, this can be done by a simple call:

AliceO2/Detectors/ITSMFT/ITS/simulation/src/digi2raw.cxx

Line 173 in b0fc846

m2r.getWriter().writeConfFile(MAP::getName(), "RAWDATA", o2::utils::concat_string(outDir, '/', MAP::getName(), "raw.cfg"));

All requests are added to the RawCreator + RawWriter is adapted.

shahor02 · 2020-08-19T10:50:50Z

Detectors/EMCAL/simulation/src/RawWriter.cxx

@mfasDa concerning the carryOverMethod: does the original data you provide to writer->addData(..) already contain the trailer (placeholder) which need to be finalized in carryOverMethod or you want to build it on the fly add and to every page payload, regardless if it the data need splitting or not?
What we discussed yesterday is to call carryOverMethod also for the last chunk if there was at least 1 splitting, but given this will be wrong if your data comes w/o trailer.
In the latter case, I would simply add an option (you will need to activate it for EMCal) to always call this method, even if splitting is not needed at all.
But if your original data contain a trailer, then it is more tricky, since it will need to be modified in the original data which is supposed to be const...

Additional point: the trailer string supplied by the method must be a multiple of 16B CRU word.
Is this is a case for the RCU trailer your create?
Also, if your original data do not contain a trailer (so it is added on top of the maxSize):
can the trailer size exceed one 16B word? In this case, if the maxSize estimated by the writer is 16B (minimal possible value), then you will get a negative actualSize.

@shahor02 Currently the trailer is added at the end of the payload, containing the full payload size which is the number of 32bit ALTRO words in the payload sent to addData. In the hardware the trailer (size typically 9 32 bit ADC words) is added to every page and it contains the number of 10 bit words per page, so in the decoding I have to add the payload size from all the trailers belonging to the same page in order to know how the exact amount of words. In the simulation the trailers must be handled in the same way.

In the carryOverMethod implementation used now I read the trailer from the end-of-payload and replace the payload size with the payload size of the page, in the same way as it is done for ITSMFT. (This answers the first question with yes) This is rather trivial as the number of 8bit words just needs to be divided by 4. However the last trailer must be adapted as well and this I can only do once I know the size of the payload in the last page.

Concerning maxSize: I was under the impression this is the size of the page? The trailer would always be smaller. Did I misunderstand something?

Hi @mfasDa
The maxSize is the number of bytes remaining in the current open page to reach 8KB limit (or the super-page to reach is limit, by default 1MB). So, ultimately, the maxSize can be just 16B. But if your data already contains the fixed-size trailer, this is not a problem. Since you are starting a new page for every trigger, I don't think you will have a situation of only 16B remaining on the 8KB page, it could happen if you add a new payload to a partially filled page. And in case of super-page filling there is a protection to not create a new page in the same super-page if less than 128B left (from which 64 will be taken by the RDH), so, you should be safe.

Basically, for the EMCAL I will need the writer will need to call carryOverMethod for the very last chunk (where no splitting is actually needed) and instead of adding the supplied trailer after writing the payload (as it would do if there is a real splitting) it should overwrite the last carryOverTrailer.size() bytes, is this correct?

@mfasDa please check if #4179 is what you want. You will need to enable the carry-over call for the last chunk by RawFileWriter::setApplyCarryOverToLastPage(true); and detect such a call as the modified test shows: https://github.com/AliceO2Group/AliceO2/blob/863c3435580f5fbe1bc34870f4a40a31f45aea82/Detectors/Raw/test/testRawReaderWriter.cxx#L166-L170

Hi @shahor02, yes, the conclusion is correct. The carryOverMethod currently sets the payload size to the trailer whenever it is called. So since the end of data contains the trailer either it has to be overwritten by the RawFileWriter or our custom carryOverMethod. In the second case I would not add a trailer but modify it myself (probably overall easier). In this case I would need to know that we are on the last page.

@mfasDa as far as I can see, your carryOverMethod sets the returned actual size to maxSize - trailer.size, this is ok except when you are on the last chunk: in this case the you should return the original maxSize as an actual payload to write and supply the modified trailer string. The writer will overtwrite last trailer.size() of the payload last chunk by this modified trailer. You cannot modify in the carry-over method the original data: it is const gsl::span, just provide an updated trailer to override the one in the payload.

OK, so I only need to know that I am on the last page to include the trailer size in the from the actual size. I was thinking to take this from the payload size but in principle the page could be 8 kB. Anything that tells me how much data is still to come? I.e. the gsl::span contains the full data or the data starting from the page?

The const gsl::span<char> data refers to the full original payload you provided to addData, const char* ptr
is start of data yet to write (can be anything between data.begin() and data.end()) and the int maxSize is the max number of bytes the writer can write (starting from the ptr) to currently open 8kb page.
You can detect that you are on the last page by

AliceO2/Detectors/Raw/test/testRawReaderWriter.cxx

Lines 166 to 167 in 72e4fba

int bytesLeft = data.size() - (ptr - &data[0]);

bool lastPage = bytesLeft <= maxSize;

shahor02

@mfasDa : you should also set (e.g. in the init) writer->setApplyCarryOverToLastPage(true) to trigger the callback at the last page.

- Replace usage of RawOutputPageHandler with RawFileWriter for output streaming - Initialize RawFileWriter using a single output file and a linear link assignment for C-RORC/Link ID untill final link assignment on production FLPs is fixed - Convert input handing from std::vectors to gsl::span

…conversion Stand alone application reading digits file and processing digits to raw conversion for each timeframe entry in the digits tree. Open points: - First IR need to be specified for methods in RawFileWriter - Raw writer settings to come from the CCDB

…h raw page CarryOverMethod cuts the payload in size - size of the RCU trailer and adds the RCU trailer from the last page, in which the payload size is adapted to the size in the page.

- Add option to split to file per full detector, subdector or crorc+link - Change rawfile to output location in order to support multiple raw files - Add parising of the configuration file in order to configure tf handling - Add writing of the output config - Remove obsolete classes

mfasDa · 2020-08-20T16:05:46Z

@shahor02 I intended to wait until #4179 is merged and then add the line in order to avoid conflicts. No I see it is merged so I will add this immediately.

…he last RCU trailer Specify also using carryOverMethod for the last page

shahor02 · 2020-08-21T11:12:41Z

@mfasDa is this ready to be merged?

mfasDa · 2020-08-21T11:17:35Z

@shahor02 Yes, please squash-merge!

mfasDa changed the title ~~[EMCAL-610] Output raw stream based on RawFileWriter~~ [EMCAL-610, EMCAL-659] Output raw stream based on RawFileWriter Jul 27, 2020

mfasDa force-pushed the EMCAL-610 branch from f663165 to 434c671 Compare July 27, 2020 15:21

mfasDa force-pushed the EMCAL-610 branch from 434c671 to 468a5dc Compare July 27, 2020 16:28

mfasDa changed the title ~~[EMCAL-610, EMCAL-659] Output raw stream based on RawFileWriter~~ [EMCAL-610, EMCAL-659, EMCAL-661] EMCAL digit to raw converter stand alone application Jul 27, 2020

mfasDa force-pushed the EMCAL-610 branch from 468a5dc to 5cb96bf Compare August 3, 2020 11:10

shahor02 requested changes Aug 10, 2020

View reviewed changes

mfasDa force-pushed the EMCAL-610 branch from 5cb96bf to 6b3e02f Compare August 18, 2020 18:24

shahor02 reviewed Aug 19, 2020

View reviewed changes

mfasDa force-pushed the EMCAL-610 branch from 0558c6c to 34452f8 Compare August 20, 2020 15:46

shahor02 requested changes Aug 20, 2020

View reviewed changes

mfasDa added 4 commits August 20, 2020 18:03

[EMCAL-610] Use carryOverMethod in order to append RCU trailer to eac…

6b51a5c

…h raw page CarryOverMethod cuts the payload in size - size of the RCU trailer and adds the RCU trailer from the last page, in which the payload size is adapted to the size in the page.

[EMCAL-610] Adapt carryOverMethod in order to adapt payload size of t…

c263d6c

…he last RCU trailer Specify also using carryOverMethod for the last page

mfasDa force-pushed the EMCAL-610 branch from 34452f8 to c263d6c Compare August 20, 2020 16:57

shahor02 approved these changes Aug 21, 2020

View reviewed changes

shahor02 merged commit da63125 into AliceO2Group:dev Aug 21, 2020

mfasDa deleted the EMCAL-610 branch August 21, 2020 11:46

	int bytesLeft = data.size() - (ptr - &data[0]);
	bool lastPage = bytesLeft <= maxSize;

Conversation

mfasDa commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mfasDa commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mfasDa commented Aug 3, 2020

Uh oh!

shahor02 commented Aug 3, 2020

Uh oh!

mfasDa commented Aug 3, 2020

Uh oh!

shahor02 commented Aug 3, 2020

Uh oh!

mfasDa commented Aug 3, 2020

Uh oh!

shahor02 commented Aug 3, 2020 via email

Uh oh!

shahor02 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!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

mfasDa commented Aug 20, 2020

Uh oh!

shahor02 commented Aug 21, 2020

Uh oh!

mfasDa commented Aug 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

mfasDa commented Jul 27, 2020 •

edited

Loading

mfasDa commented Jul 27, 2020 •

edited

Loading