Skip to content
Snippets Groups Projects

new unpacker for the trd2d.v06.24 data format

Merged Alexandru Bercuci requested to merge a.bercuci/cbmroot:2d-unpacker24 into master

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.
Edited by Alexandru Bercuci

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Looks fine for me apart from the small fix in description (cannot claim that I understood all details :sweat_smile:)

    • 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

  • Alexandru Bercuci changed the description

    changed the description

  • 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 1 commit

    • 9c4560a0 - update doc for the new functionality

    Compare with previous version

  • Alexandru Bercuci resolved all threads

    resolved all threads

  • added 1 commit

    Compare with previous version

  • Pierre-Alain Loizeau approved this merge request

    approved this merge request

  • Pierre-Alain Loizeau enabled an automatic merge when the pipeline for 5df71b2d succeeds

    enabled an automatic merge when the pipeline for 5df71b2d succeeds

  • Alexandru Bercuci resolved all threads

    resolved all threads

    • @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 the sys_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?

    • Please register or sign in to reply
  • 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

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading