Skip to content

GitLab

  • Menu
Projects Groups Snippets
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in
  • L LHCb
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 163
    • Issues 163
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
  • Jira
    • Jira
  • Merge requests 66
    • Merge requests 66
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Issue
    • Repository
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • LHCb
  • LHCb
  • Issues
  • #165

Closed
Open
Created Oct 21, 2021 by Rosen Matev@rmatevMaintainer0 of 6 tasks completed0/6 tasks

Follow-up from "New calorimeter encoding/decoding"

The following discussions from !3287 (merged) should be addressed:

  • @cmarinbe started a discussion: (+1 comment)

    why is this still a GaudiAlgorithm and not a Transformer?

  • @cmarinbe started a discussion:

    Since not any positive integer is valid, I think we should still check that the one set here is. We might need a new variable that holds all the valid coding types and then checks that m_dataCodingType is within those. Or change the current coding type 10 to 4 so all the valid types are consecutive and then just check here that m_dataCodingType <= 4. Is there any particular reason to prefer 10 over 4 for the new type?

  • @cmarinbe started a discussion: (+1 comment)

    this code is identical for each coding type, except for the bit setting m_numberOfBanks. Could we move all the rest outside the ifs to avoid code repetition and make this much more readable?

  • @cmarinbe started a discussion: (+1 comment)

    why don't we have a case 3 here?

    Actually, looking at the rest of the code it seems we only differentiate between type 10 and the rest, so in practice case 2 and 3 look the same to me (assuming they would do the same here as well). Could you clarify if we really need a case 3 or 2 is already the same?

  • @jmarchan started a discussion: (+1 comment)

    I modified the code to handle both little endian and big endian since big endian will be used for beam tests, little endian has version 2 and big endian has version 1.

    The code has been tested on some files generated with the 2 configurations: /eos/lhcb/user/j/jmarchan/Boole-Extended-NewDecoding-LittleEndian-20211020.digi/mdf /eos/lhcb/user/j/jmarchan/Boole-Extended-NewDecoding-BigEndian-20211020.digi/mdf

  • @rmatev started a discussion:

    we should revisit the lookup with find_if as it feels a bit unnecessary and the same pattern repeats too many times.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking