-
Notifications
You must be signed in to change notification settings - Fork 485
TRD fix for empty frames #6511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TRD fix for empty frames #6511
Conversation
429acb8 to
cdb1d5b
Compare
16c02cf to
7dd216f
Compare
1abc0d5 to
37ebdd3
Compare
|
@shahor02 if you are happy can we merge this once the ci is finished, although i am yet to get it all green, with failures in other places. |
37ebdd3 to
b09dce2
Compare
| //auto datadesc = (mUserDataDescription == o2::header::gDataDescriptionInvalid) ? o2::header::gDataDescriptionRawData : mUserDataDescription; | ||
| auto datadesc = (mDataDesc == o2::header::gDataDescriptionInvalid.str) ? o2::header::gDataDescriptionRawData : mUserDataDescription; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where do you set the mUserDataDescription (except initialization by gDataDescriptionInvalid) but what do you need it at all? Don't you have all needed info in the mDataDesc ? And I don't see how do you use at all this datadesc? You still hardcode the RAWDATA.
Could you explain in which cases the input will not be the RAWDATA, what it is will be and who will handle the RAWDATA in this case (I guess the StfBuilder always sends the RAWDATA?) ?
Concerning the treatment of trd-datareader-inputspec option: in the DataReader you are treating it is a global workflow option, in the DataReaderTask.cxx: as a local device option. This may pose a problem since in general these are different type of options. Please don't mix them, since it is in reality a workflow option (defined in DataReader.cxx) and you are parsing it there, you better simply pass its parsed value to DataReaderTask, as you do for other arguments in
| algoSpec = AlgorithmSpec{adaptFromTask<o2::trd::DataReaderTask>(compresseddata, byteswap, verbose, headerverbose, dataverbose)}; |
Finally, we are placing the workflows and DPL divices classes in separate workflow directory, could eventually (in different PR) move DataReader and DataReaderTask to such dir.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original idea was to have RAWDATA and CRAWDATA for without and with the compressor. Will clean up now and only do RAWDATA, the parameter list kept failing for a string or char* or various other things i tried, so simply reverted to putting the option code you see in the init. Will simply change it all to RAWDATA and remove the inputspec option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, because if you would expect the CRAWDATA, the protection would need to be moved to the processor which converts RAWDATA to CRAWDATA. The point is to have a protection at the 1st instance where the TF might be missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool that will go in another pr
| void unpackDataForSending(std::vector<TriggerRecord>& triggers, std::vector<Tracklet64>& tracklets, std::vector<Digit>& digits); | ||
| void sendData(o2::framework::ProcessingContext& pc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I understand correctly that you fill the vectors twice, once in the unpackDataForSending and also in sendData?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unpackDataForSending is just for debugging, the production version only uses sendData. It was easier to do it that way when i wanted to get access to all of vectors. I renamed it now to simpy unpackData
bac8287 to
6979ae7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, that you have hardcoded RAWADAT, does this mCompressedData make sense?
Which device is going to send compressed data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to make it full proof, one has to start the datareader task with a command line of option of trd-datareader-compresseddata so the added safety of a different data type is not really required. You feel it should be done differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain where the compressed data are coming from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datareader connects to crurawreader and compressedrawreader depending on the origin of its data.
So the CompressorTask uses the crurawreader and the DataReaderTask uses both depending data origin.
crurawreader-CompressorTask-send compressed to epn.
crurawreader/compressrawreader - DataReaderTask -- digits/tracklets/index--> ctf
my ascii art foo is weak.
293459d to
665b754
Compare
|
@shahor02 could this be merged? 2 other prs need to be rebased/merged to this pr |
Fix trd raw data for empty frames.