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 aTransformer
? -
@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 thatm_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 theif
s 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.