Skip to content

speed up and fix up trd raw data reading. - #6799

Merged
shahor02 merged 8 commits into
AliceO2Group:devfrom
bazinski:parsezerosuppressed
Aug 11, 2021
Merged

speed up and fix up trd raw data reading.#6799
shahor02 merged 8 commits into
AliceO2Group:devfrom
bazinski:parsezerosuppressed

Conversation

@bazinski

@bazinski bazinski commented Aug 3, 2021

Copy link
Copy Markdown
Collaborator

fix parsing of zero suppressed digits.
increase verbose output
fix bitcounting error causing slow readout
other speed ups, 7 hours per timeframe now down to .7ms per timeframe.
byteswap the adc bit pattern word
fix the DigitMCMADCMask it was reversed.

attempt to jump the non tracklet data at the beginning of the tracklet parse block, partially successful.
erroneous additional digits are marked with EEE in info output

change datareader to write out triggers with the digits (temporary most likely)

This could well change depending on the testing tomorrow.

@tdietel tdietel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's still work in progress. I made some comments, but I'm happy to merge if it helps with progress.

uint32_t adcmask : 21;
uint32_t j : 4; // unused always 0xc
uint32_t adcmask : 21;
uint32_t c : 5; // unused always 0x1f

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the comment correct? I thought it's the inverted number of ADC channels with data.

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.

correct as to my understanding at time of writing, but no not correct.

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.

now fixed.

Comment thread DataFormats/Detectors/TRD/src/RawData.cxx
return 22;
int position = channel;
int m = 1 << channel;
while (!(bp & m)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this code do the same as:

Suggested change
while (!(bp & m)) {
for (int pos=channel+1; pos<22; pos++) {
if ( bp & (1<<pos) ) {
return pos;
}
}
return 22;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure what your code does once it's at (or rather beyond) the last bit.

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.

the newer one works properly, verified. unit tests to come.
the bit is fliped so that each time is simply looks for the next right most bit with a 1 in it, so at the end one has no bits and has a condition for finishing.

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.

To be clearer line 447,448 handles the case of no bits valid, it is possible to start on the left of the left most bit.
adding a termination at position 32;

Comment thread Detectors/TRD/reconstruction/src/DigitsParser.cxx Outdated
@@ -270,80 +343,77 @@ int DigitsParser::Parse(bool verbose)
}
if (digitwordcount == constants::TIMEBINS / 3) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this actually work if the number of timebins is not divisible by 3?

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.

it works in the sense that it wont write a digit, but carry on.

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.

just above that there is a check for digitwordcount > timebins/3 with a fatal log message.

LOG(info) << "adcmask: 0x" << std::hex << mADCMask << " and channel : " << std::dec << mChannel;
}
mChannel = nextmcmadc(mADCMask, mChannel);
if (mChannel == 21) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought I saw a 22 up there...

@bazinski bazinski Aug 3, 2021

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.

21 and 22, 22 gives logs the erroneous digit, and 21 should never get hit, there as a safety net should maybe change it to >21

@bazinski
bazinski force-pushed the parsezerosuppressed branch 2 times, most recently from 3fbc1f4 to 53685a5 Compare August 9, 2021 23:36
@bazinski
bazinski force-pushed the parsezerosuppressed branch from 53685a5 to 9c66e87 Compare August 10, 2021 13:42
@bazinski

Copy link
Copy Markdown
Collaborator Author

should i leave the list of updates or collapse them all into 1? this can then be merged, stats will come shortly in a seperate pr @tdietel

@shahor02

Copy link
Copy Markdown
Collaborator

@bazinski I can squash them at merging, shall I already do this?

@tdietel

tdietel commented Aug 11, 2021

Copy link
Copy Markdown
Contributor

Squashing looks like a good idea. Shall we go ahead and merge?

@shahor02 shahor02 left a comment

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.

squashing/merging

@shahor02
shahor02 merged commit 2fa3ba2 into AliceO2Group:dev Aug 11, 2021
@bazinski
bazinski deleted the parsezerosuppressed branch May 26, 2022 18:40
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