Iss952 Fix Z position of ECal Face

[normangraf]

There were a lot of instances in our code base of the static methods used to propagate a track to the face of the Ecal. I have tested the code and verified that it works correctly when reconstructing data from 2016, 2019 and 2021. It could use a thorough review to make sure that I did not introduce any bugs.

[pbutti]

Hi Norman,

thank you for taking care of this.
The change seems OK as a first pass. I would prefer using a DB to store the ECAL position but it's ok. The only comment I have is:

TrackUtils.RunPeriod runPeriod = TrackUtils.RunPeriod.PhysRun2021; if (4441 < runNumber && runNumber < 5967) { runPeriod = TrackUtils.RunPeriod.EngRun2015; } if (7219 < runNumber && runNumber < 8100) { runPeriod = TrackUtils.RunPeriod.EngRun2016; } if (9001 < runNumber && runNumber < 10740) { runPeriod = TrackUtils.RunPeriod.PhysRun2019; } if (14131 < runNumber && runNumber < 14775) { runPeriod = TrackUtils.RunPeriod.PhysRun2021; }
This code snippet is duplicated in many places. I suggest to change it to a single function in trackUtils or change the extrapolation to the ECAL that takes the run number and the ECAL-extrapolator checks itself the period. In this way we can remove this code snippet from many places.

[normangraf]

I concur that it's not a very elegant solution, which Maurik has also pointed out. The code has been copied and pasted simply because I kept finding odd places where our code was accessing the ECal face. I have a better idea how to implement this, but do not have the time to implement it at the moment. I would ask to proceed as is and clean things up in a later refactoring.

[cbravo135]

This PR has a lot of style changes included which makes it difficult to review. Please remove style changes to make review process easier. Wouldn't this less elegant solution be much easier to just pass the run number to the utils and resolve the run period there, only in a single place? Kinda unelegant squared, if we are just going to use something unelegant for now, might as well make it the easy way and not this convoluted way.

[mholtrop]

While not perfect, neither is the rest of our code, but it works.
We should probably make an issue for a cleanup pass, which can include the reduction of the number of repeated code blocks.

[cbravo135]

Sad excuse to not make a 10 minutes change to improve the code drastically. Will open a PR shortly.