Follow-up from "Add workaround in FT decoding for RawBanks incorrectly labeled as v7 when infact they are v8"

The following discussion from !4210 (merged) should be addressed:

The MR above implements a work around for FT to decode data labelled as v7 as v8, which is what the data really is. The work around is enabled according to the affected run number range.

However, at the time it was implemented the end run was not know, as the firmware was not fixed. As soon as this is done the end affected run should be updated from its current ‘big number’ to the actual last affected run.

@lohenry @ldufour @rmatev FYI

assigned to @lohenry and @ldufour

changed the description

added Decoding FT labels

Are there already some thoughts about how this will be dealt with in simulation for 2023 conditions?

In the simulation Boole already puts the correct version (8) in the raw banks.

The only case to worry about is incorrectly interpreting v7 MC as v8. We discussed this with @lohenry briefly but the MR was merged before we could add a protection for this.

I think odin.partitionID() is never set for MC (so it defaults to 0) and is never 0 in data so we can add a check odin.partitionID() != 0 and a comment to explain that this is so that the hack is disabled on MC.

I think that the goal is more to make sure we have the same mistake in for HLT1, for 2023 simulation - it should reflect how we took data.

I see... this gets more complicated. If we agree that HLT2 will be run on data with the workaround (to decode v7 as v8), then I would suggest that we add an option to the HLT1 decoding to decode v8 as v7, and we only set this for this MC step. Any drawbacks?

I don't see an immediate drawback.

@lohenry Could you prepare such an option, to introduce the bug in MC for HLT1 (and possibly also for HLT2, as we have processed data with the bug in HLT2)?

Sorry, yes! I'll get to it today hopefully.

Finally got around to it, pushed a yet-untested Allen branch: Allen!1288 (merged)

mentioned in merge request Allen!1288 (merged)

@lohenry @ldufour Is it known yet what the last run that needed the decoding work around is, i.e. has the fix to the version been deployed in the firmware yet ? If so please do not forget to update the run range here accordingly

https://gitlab.cern.ch/lhcb/LHCb/-/blob/master/FT/FTDAQ/src/FTRawBankDecoder.cpp?ref_type=heads#L757

On it, thanks!

mentioned in merge request !4273 (merged)

closed with merge request !4273 (merged)

mentioned in commit a079afd5

mentioned in merge request !4284 (merged)

mentioned in merge request Moore!3032 (merged)