Feature seg truth hotfix

[YifanC]

No description provided.

[krwood]

Do you have any validations you can show? Reading through by eye, it's very difficult to follow that various *tracks datasets, as things seem to have gotten rather disorganized. I don't love the fact that we are drifting and quenching and swap_coordinates different *tracks datasets multiple times in different places. There are nested statements that could rewrite one to be the other in under certain conditions... it's gotten very difficult to follow the code and I worry this makes us prone to introducing bugs, especially as we rely on bringing in new developers. I can get more specific and turn this into an issue, but for this PR it would be nice to at least see some validations that the complicated logic is being handled properly for our current configs.

[mjkramer]

I agree with Kevin's general sentiment, but this hotfix seems pretty safe to me. It just adds missing info (n_electrons, t_start, etc.) to the segments dataset that we write to disk. A "why do we do this" comment in the code would be nice, though.

[YifanC]

@mjkramer @krwood As Matt suggested, I added comments on why we need the hotfix and why it is a hotfix.
It is a fairly isolated change. all_mod_tracks are passed in the module loop as tracks which have the truth information updated, but not all_mod_tracks, we are patching all_mod_tracks here because the unsolved flow issue.
The code is tested to a degree that it verifies the output contains the updated segments.

[YifanC]

Oh btw I updated the Git Issue #216 @krwood

[krwood]

I will begrudgingly approve the PR, but I’m still not comfortable with the way the code is laid out. Specific to this PR, I really don’t think it’s great that we simulate the quenching and drifting on one segment dataset, and then simulate the pixel responses, and then re-simulate the quenching and drifting in another segment dataset for “back tracking”. We should backtrack to the exact input to the pixel response simulation (especially with the outstanding reproducibility issues). I wouldn’t be surprised if we find the backtracking bugs you and Kazu are encountering in the ml reco training work are related to this.

[YifanC]

I will begrudgingly approve the PR, but I’m still not comfortable with the way the code is laid out. Specific to this PR, I really don’t think it’s great that we simulate the quenching and drifting on one segment dataset, and then simulate the pixel responses, and then re-simulate the quenching and drifting in another segment dataset for “back tracking”. We should backtrack to the exact input to the pixel response simulation (especially with the outstanding reproducibility issues). I wouldn’t be surprised if we find the backtracking bugs you and Kazu are encountering in the ml reco training work are related to this.

Relate this comment to Kevin's Git Issue #228