new unpacker for the trd2d.v06.24 data format
Extension of the FASP data unpacker. The unpacker support both the old (legacy) format used until 03.2024 and the new data format (valid from 06.2024) shipped with new FASPRO HW/FW. The following is a list of changes:
- the bit meaning in the FASP word (32bit) was reordered
- the need for a second loop on the digi to reasign ADC values was removed (still kelp in the code for backward compatibility) so the code is much simpler.
Merge request reports
Activity
requested review from @p.-a.loizeau
assigned to @p.-a.loizeau
added Reconstruction TRD2D mCBM labels
- Resolved by Alexandru Bercuci
- Resolved by Alexandru Bercuci
Not sure how far it derived from this version but if you want to have a look at the online version it is at
algo/detectors/trd2d/
.
I get the feeling from a quick look that some of the logic was changed to generate the mapping once at startup, so not sure how easy it will be to adapt without expert help.In any case: I ping @d.smith here so he has the reference of the changes when he comes back in 2 weeks
Dear @a.bercuci, @p.kaehler,
you have been identified as code owner of at least one file which was changed with this merge request.
Please check the changes and approve them or request changes.
added CodeOwners label
mentioned in merge request CbmSoft/cbmroot_parameter!195 (merged)
- Resolved by Alexandru Bercuci
Setting now to auto-merge, we can keep the online version for a second MR
enabled an automatic merge when the pipeline for 5df71b2d succeeds
@a.bercuci @p.-a.loizeau Ok, I have taken a look at these changes. It appears to me that this is not super easy to port to the online code, but also not super hard, so somewhere in between.
My immediate question is, what are the plans for carrying out the port? By merging this, we have certainly accrued some amount of technical debt, and this should be rectified as soon as possible. I would like to avoid the situation where offline and online codes diverge strongly at all cost.
Also, it would in general (not just in this case, but also in others) be nice if the updates to the online code would be done directly by the detector groups. Otherwise we are stuck in an endless cycle of playing catch-up.
There is also a structural question, for which input from @fweig and @v.friese is desirable: In the version of the code merged here, switching between different versions of the message format is done by an if-condition inside the unpacker. This is possible here, because the messages themselves apparently carry a version flag. But ultimately, we want to avoid this, and instead have separate unpacker classes for each message format, right?
The prevalence of version-switching clauses was a big source of code entropy in the past.
There is also a structural question, for which input from @fweig and @v.friese is desirable: In the version of the code merged here, switching between different versions of the message format is done by an if-condition inside the unpacker. This is possible here, because the messages themselves apparently carry a version flag. But ultimately, we want to avoid this, and instead have separate unpacker classes for each message format, right?
From what I remember originally the plan was to do the switching using the format/version flag
sys_ver
in the microslice descriptor, therefore allowing to have an if condition only in the top layer of the unpacker code (close to where we switch based on thesys_id
) and completely separate unpacking algorithms.
Not sure however if this field was bumped on the FW side for this beamtime, maybe @a.bercuci or @schiaua_AT_nipne.ro can comment here?
Also, it would in general (not just in this case, but also in others) be nice if the updates to the online code would be done directly by the detector groups.
I fully agree with this view. Although it is an extra burden on my shoulders I would prefer to have control over the online code too.
So once you answer on the question :
switching between different versions of the message format is done ...
I would proceed with the online implementation and than we can merge them together