Fix DD4HEP sensor ID in mchit not being the same as detdesc for VPClusterEffSimDQ

The code crashes when reading dd4hep MC as there is an assumed sensor numbering. This is the same issue addressed in Boole!461 (merged) but for the VPClusterEffSimDQ algorithm.

added VP label

requested review from @tlatham

added 1 commit

• 4632034c - Lb-format fixes

Compare with previous version

added RTA label

Many thanks @hcroft. You've also included the changes from !763 (merged) here by the looks of things, why is that? Should we close that MR? I'm not sure, following the recent fixes in Detector, whether the workaround in !763 (merged) is still needed. Can you comment on that? You mention a version of Gauss producing inconsistent IDs in the description but I thought the problem was due to code in Boole being affected by the bug, not Gauss.

Anyway, I have one comment on that part (which maybe I should also add to !763 (merged)?) and another very minor one on the new stuff added in this MR but otherwise this looks good to go and I gather solves the problem @seaso reported, which is great.

added 1 commit

• 55db5dd4 - Apply 1 suggestion(s) to 1 file(s)

Compare with previous version

@tlatham Yes I had included !763 (merged) mostly as my copy of the code had that in and I wanted to get this out quickly. I will remove it from here, as to if it should be deleted, there exists MC for which is it necessary, MC generated before the other fix (was included. but no big productions so most people will not notice. I did but that is due to seeing Boole metrics showing the detector had no efficiency in a nightly test sample. I'll try and tidy up the MR today.

added 2 commits

• 1981e6c0 - Revert "Use mask to set VPChannelID bits (suggested code change)"
• 983e3355 - Tidy up includes

Compare with previous version

added 1 commit

• d810c9e0 - Fixed code to use MCHit SensID where value is not sensor number properly

Compare with previous version

Cleaned up MR to not also include !763 (merged) and applied changed suggested by @tlatham to simplify includes.

resolved all threads

marked this merge request as ready

changed the description

mentioned in issue Moore#896 (closed)

mentioned in merge request !763 (merged)

Thanks David, I think this looks good to go - @seaso can you just re-check that this still works for you?

/ci-test

added ci-test-triggered label