The suggestions from the meeting were:

• Change the type name to a more neutral (description could be changed too, we of course can point to the suggested type of use in charge sharing)
• We should check if there is a way to recover track-truth association fractions when using this type in the reconstruction codes
• Alternatively we could consider introducing ChargeSharedSimHits (maybe with different info). Dummy charge sharing factory could be implemented for use with existing detectors (SVT, MPGD), then factories can be changed to consume that type and do the additional indirection when doing truth lookup.

cc @ruse-traveler @ShujieL @wdconinc @ybedfer

I changed the name of the class to SimTrackerHitLink.

I have been thinking about ways to recover the one-step association. One approach would be to create another class dedicated to truncating the intermediate links, so that the raw hits link directly to the underlying sim hit in a single step. However, even in that case, this class would still need input from the LGADChargeSharing class to determine which sim hit was spawned by which other sim hit, so we would still need the sim-to-sim hit link regardless.

One way to eliminate the sim-to-sim hit link completely would be to modify the LGADChargeSharing algorithm so that, instead of creating SimTrackerHits from SimTrackerHits, it creates charge-shared RecHits from a single (non charge-shared) RawHit. Then we could simply use SetRawHit on the charge-shared RecHits to link back to the original RawHit. However, I am not sure this is a good idea, because the charge-sharing class needs the true sub-pixel hit position information to estimate the degree of charge sharing. Furthermore, creating a RecHit at this stage may not be ideal, since we have plans for a more sophisticated digitization routine downstream. We may want additional processing on the output of the charge-sharing class before arriving at the final RecHits.

My opinion is that, unless we change the input/output class types of the LGADChargeSharing class, a sim-to-sim link is necessary, regardless of where the links are later processed to obtain the reco-truth association in a single step.

SVT had noise injection at Raw hits stage eic/EICrecon#2670. Is charge sharing at raw hit stage also possible? Then they can have a one-to-many relation to the corresponding sim hits (can be more than one hits b/c the two clusters may overlap).
As long as we preserve the track --> hit --> particle relation chain, I think it's OK to add intermediate steps and links to support detector studies and analysis.

SiliconChargeSharing outputs SimTrackerHits because, at least at that time, our group wants charge sharing to be an optional class that can be taken out without affecting downstream classes that takes SimTrackerHits by default. At that time, PulseGenerator which is immediately downstream of Charge Sharing class only takes SimTrackerHits as input, so my hand was tided.

But things have changed now. PulseGenerator do take other classes as input. I guess I can give Shujie's idea of generating raw-hits a try.

Alternatively we could consider introducing ChargeSharedSimHits (maybe with different info). Dummy charge sharing factory could be implemented for use with existing detectors (SVT, MPGD), then factories can be changed to consume that type and do the additional indirection when doing truth lookup.

Personally, I like @veprbl's suggestion to implement dummy factories for the other detectors. This is functionality we'll ultimately have to implement for all of them, right? If so, then I think it makes sense to put in placeholders with the links!

Alternatively we could consider introducing ChargeSharedSimHits (maybe with different info). Dummy charge sharing factory could be implemented for use with existing detectors (SVT, MPGD), then factories can be changed to consume that type and do the additional indirection when doing truth lookup.

Personally, I like @veprbl's suggestion to implement dummy factories for the other detectors. This is functionality we'll ultimately have to implement for all of them, right? If so, then I think it makes sense to put in placeholders with the links!

This is similar to what we did with SVT noise. SimHits --> RawHits with noise (is a dummy collection if the noise is off) --> RecHits.

SVT had noise injection at Raw hits stage eic/EICrecon#2670. Is charge sharing at raw hit stage also possible? Then they can have a one-to-many relation to the corresponding sim hits (can be more than one hits b/c the two clusters may overlap). As long as we preserve the track --> hit --> particle relation chain, I think it's OK to add intermediate steps and links to support detector studies and analysis.

I gave it a lot of thought. I thought this was a good idea so I was half way done implementing it before running into some difficulties that changed my mind.

In my pull request for TOF clustering, SiliconTrackerDigi class comes right after SiliconChargeSharing class. SiliconTrackerDigi turns SimTrackerHits into a RawTrackerHits, so making SiliconChargeSharing outputs RawTrackerHits seems to be no-brainer, as all I need to do is to skip the SiliconTrackerDigi entirely.

But the problem is that in some other detectors like LowQ2, the output of charge sharing is piped into PulseGenerator, which takes a SimTrackerHit. I tried to write an adaptor in PulseGenerator to accept RawHits, but I don't find a satisfying way to write a PulseGenerator::addRelations. There is no "MutablePulseType::addToRawHit" interface yet. While I can implement one, I doubt having a mix of raw hits and sim hits relation simultaneously is a good idea, since RawHits and SimHits maybe duplicates of each other. Eventually, TOF will also switch to using PulseGenerator eventually so compatibility with PulseGenerator can't be ignored.

The same problem arises with creating a SimChargeSharedHits class. Even if it shares the same content as SimTrackerHits, I will still have to add interfaces for PulseGenerator to accept it and for MutablePulseType to accept SimChargeSharedHits as one of its relatable type.

Modifying all interfaces and Pulse schema seems to be more involved than I originally hope for. I can still do it, but I would like to ask for @veprbl and @ShujieL's opinion before committing to either solution. Should I change Pulse to add relation with raw hits? Or should I create SimChargeSharedHits and include relation with pulse class? Or if there are better alternatives, please don't hesitate to let me know.

I don't think we want to do charge sharing on raw hits as they are supposed to represent already digitized quantities that we will see out of the detector itself.

My opinion is we should add the SimTrackerHitLink for now, I don't think there is any harm in having it available.

Then moving on we could create a new hit type which represents the analogue parameterization containing the information which is required from the charge sharing to be able to independently reconstruct an accurate pulse in a pixel (or skip to ASIC dependant digitized hit). This hit type will require extra thought but I don't think the SimTrackerHit is the correct long term solution.

I can also develop a new datatype as the output of charge sharing. However, it is still a mildly complex projects as,

1. Need to change SiliconTrackerDigi to accept either SimTrackerHits and the new class as input. I can make it a template like PulseGeneration.
2. Need to add relation between this class and the Pulse class, i.e. "MutablePulseType::addToChargeSharedHit" something like that.

Or, if we just want a minimal make shift solution, have charge sharing output the new data type, then I write an adapter class that immediately converts it back to SimTrackerHit so SiliconTrackerDigi can accept it without changing much of existing code. The immediate concern is that it renders the new class pointless.

Or, just as @simonge suggested, we can accept this Sim -> Sim link for now and think about a solution later.

I am open to all ideas, but I do need agreement from the experts before I commit to any idea.

I can also develop a new datatype as the output of charge sharing. However, it is still a mildly complex projects as,

1. Need to change SiliconTrackerDigi to accept either SimTrackerHits and the new class as input. I can make it a template like PulseGeneration.
2. Need to add relation between this class and the Pulse class, i.e. "MutablePulseType::addToChargeSharedHit" something like that.

Or, if we just want a minimal make shift solution, have charge sharing output the new data type, then I write an adapter class that immediately converts it back to SimTrackerHit so SiliconTrackerDigi can accept it without changing much of existing code. The immediate concern is that it renders the new class pointless.

Or, just as @simonge suggested, we can accept this Sim -> Sim link for now and think about a solution later.

I am open to all ideas, but I do need agreement from the experts before I commit to any idea.

Personally, I agree with @simonge on this: I think it's reasonable to accept the sim-to-sim link now since there's no harm in making the type available.