Remove unused hits columns

[gonzaponte]

There are several columns that hold dummy values due to historical reasons. This PR removes them and breaks backward compatibility to reduce noise in the output file and save memory. These columns are:

• nsipm (always 1)
• Xrms / Yrms (always 0)
• Qc (always -1)
• track_id and Ep (always -1, pre-paolina)

[Luismiguelvillar]

Hi Gonza! I have a qestion regarding this PR. I have been looking into event_model.py and still many of the unused columns are still there, for example in the HitCollection class definition. Is there a particular reason for this?

[gonzaponte]

In principle, no, just leftovers that I didn't pick up in my work. Well spotted, I'll work on removing them.

[Luismiguelvillar]

Make sense! There are many "hidden" references across all IC hahaha. I also found a nsimp reference at:https://github.com/gonzaponte/IC/blob/df53e196cb19900463791becec789f29d9a33113/invisible_cities/reco/psf_functions.py#L107C5-L107C30

[gonzaponte]

Thanks! I will search the entire repo for the unused columns.

[Luismiguelvillar]

Thanks! I will search the entire repo for the unused columns.

Perfect! Sounds good! I also found this reference in buffy: https://github.com/gonzaponte/IC/blob/df53e196cb19900463791becec789f29d9a33113/invisible_cities/cities/buffy.py#L112C1-L112C63

[Luismiguelvillar]

I also found this in eutropia.py: https://github.com/gonzaponte/IC/blob/df53e196cb19900463791becec789f29d9a33113/invisible_cities/cities/eutropia.py#L81

[Luismiguelvillar]

Make sense! There are many "hidden" references across all IC hahaha. I also found a nsimp reference at:https://github.com/gonzaponte/IC/blob/df53e196cb19900463791becec789f29d9a33113/invisible_cities/reco/psf_functions.py#L107C5-L107C30

I also saw that nsimp is used in trude.py

[gonzaponte]

I've removed the remaining references to the variables we are discussing, with the exception of those that use the Cluster class directly. I think that bit of work belongs to the removal of the event model as a whole.

The other nsipm references you may encounter are not related to hits, but to other bits of the processing chain.
Same goes for X/Yrms which is used differently in the kdst stuff.

[Luismiguelvillar]

I've removed the remaining references to the variables we are discussing, with the exception of those that use the Cluster class directly. I think that bit of work belongs to the removal of the event model as a whole.

The other nsipm references you may encounter are not related to hits, but to other bits of the processing chain. Same goes for X/Yrms which is used differently in the kdst stuff.

Great!! I have a quick question. I was looking at the function track_blob_info_creator_extractor()
(https://github.com/gonzaponte/IC/blob/4f448e2579a326c30df9f66e3a6335a66d47369c/invisible_cities/cities/components.py#L1454
) and I’m wondering what the impact would be of removing Ep from HitCollection. This function appears to use Ep explicitly, so I want to make sure I understand how that dependency would be handled after the change.

[gonzaponte]

The Ep field is generated within the cities (esmeralda and isaura). I guess the question is whether we want to keep it in the output. You and @jwaiton are probably the most likely users, I will ask around here as well if anyone uses this quantity.

[Luismiguelvillar]

The Ep field is generated within the cities (esmeralda and isaura). I guess the question is whether we want to keep it in the output. You and @jwaiton are probably the most likely users, I will ask around here as well if anyone uses this quantity.

Yes, we use Ep at the moment, but changing the algorithms to not rely in Ep is straightforward!

[Luismiguelvillar]

The Ep field is generated within the cities (esmeralda and isaura). I guess the question is whether we want to keep it in the output. You and @jwaiton are probably the most likely users, I will ask around here as well if anyone uses this quantity.

Yes, we use Ep at the moment, but changing the algorithms to not rely in Ep is straightforward!

I think it will be a good idea to eliminate Ep completly, but maybe @jwaiton has something to say!

[gonzaponte]

When you say eliminate, you mean parametrize the algorithm to be able to choose which one to use? If that's the case I'm ok with that, but that's orthogonal to this PR.

[gonzaponte]

By the way, Ep is used internally to be able to modify the hits' energy in the track-finding algorithm without affecting the event energy.

[Luismiguelvillar]

When you say eliminate, you mean parametrize the algorithm to be able to choose which one to use? If that's the case I'm ok with that, but that's orthogonal to this PR.

Fair enough!

[Luismiguelvillar]

By the way, Ep is used internally to be able to modify the hits' energy in the track-finding algorithm without affecting the event energy.

Yes, I understand! In that case it makes sense! I will have a look at the rest and ask more questions when appropiate!

[jwaiton]

By the way, Ep is used internally to be able to modify the hits' energy in the track-finding algorithm without affecting the event energy.

As far as I can tell (I could be very wrong), within the compute_and_write_tracks_info() code, there is no moment in which the Ep is explicitly modified, only called as reference. I have some issues with the way Ep is assigned as well, but as you said this is orthogonal to this PR :)

[Luismiguelvillar]

By the way, Ep is used internally to be able to modify the hits' energy in the track-finding algorithm without affecting the event energy.

As far as I can tell (I could be very wrong), within the compute_and_write_tracks_info() code, there is no moment in which the Ep is explicitly modified, only called as reference. I have some issues with the way Ep is assigned as well, but as you said this is orthogonal to this PR :)

That is what I thought, maybe we can look into this in the near future!

[gonzaponte]

As far as I can tell (I could be very wrong), within the compute_and_write_tracks_info() code, there is no moment in which the Ep is explicitly modified,

I remembered that the reason for the existence of Ep was that there was something in there that needed to modify the energy. It took me a while, but I think I've found it. It's this line in the voxel-dropping function.

[Luismiguelvillar]

Everything looks fine and all doubts have been answered. I approve the pull request

[gonzaponte]

Thanks, @Luismiguelvillar. For future reference, we don't merge-commit like this. We let a third person do it (to increase the know-how) following the instructions in this file. I'll rewrite the history now.