cta-observatory / cta-lstchain

LST prototype testbench chain
https://cta-observatory.github.io/cta-lstchain/
BSD 3-Clause "New" or "Revised" License
24 stars 77 forks source link

Defined range of az_tel for real data and simulation data #1079

Closed SeiyaNozaki closed 3 months ago

SeiyaNozaki commented 1 year ago

I realized the defined range of az_tel is different between real data (-90 < az_tel < 270) and simulation data (0 < az_tel < 360). Is it a known feature? I think it is not good when using az_tel (instead of sin_az_tel) for random forest training.

moralejo commented 1 year ago

That is worrying... Did you see that on a specific sample?

SeiyaNozaki commented 1 year ago

First, I found the azimuth value of run 5466 is negative. Then, I see only positive azimuth in merged training DL1 data. Actually, the azimuth of the park in position is -1.36 degrees in the drive log file. So, I think it is not the specific case, but the definition itself is different between real and simulation data.

maxnoe commented 1 year ago

As far as I know,

(-90 < az_tel < 270)

is the real range of the azimuth drive, (i.e. the real movement of the telescope can happen in these bounds due to how the cabeling etc. works)

The range in simulations is of independent of any hardware constraints of the real telescope.

I am closing this here, because I think reporting the actual values delivered from the drive is the correct thing to do for the IO code.

The preprocessing code in lstchain for the machine learning should instead take care of this. This can be achieved by using astropy.coordinates.Angle.wrap_at

E.g. to normalize all azimuth angles to the range [0, 360°), use:

az = Angle(az, u.deg).wrap_at(360 * u.deg).deg

To get it in the range [-180°, 180°), use

az = Angle(az, u.deg).wrap_at(180 * u.deg).deg
rlopezcoto commented 1 year ago

@SeiyaNozaki did you (or anybody) seen anything worrying back when we were using az_tel instead of sin_az_tel apart from the discontinuity in 0,360 that you reported some time ago? I think we are safe now that we are using sin_az_tel, but to be on the safe side we could wrap the angles to 360 deg as @maxnoe is suggesting

moralejo commented 1 year ago

Luckily this affects mostly north-culminating sources, and for them the variation of performance with azimuth is minimal because the orthogonal component of the B-field is within ~10% of its maximum for all directions. This is likely the reason why this has gone unnoticed.

moralejo commented 1 year ago

@SeiyaNozaki shall we give a try at the wrapping? Or perhaps better, we could make the calculation of the sin(az) inside the DL1 to DL2 stage (for DL1 files where it does not exist, i.e. all our standard ones now), and stop using az in the RFs.

SeiyaNozaki commented 1 year ago

yeah, at least it would be better to wrap azimuth to avoid any confusion. It is also good to calculate sin(az) inside the DL1 to DL2 stage, but if you want to know if sin(az) is really needed, I can check.

moralejo commented 1 year ago

Well, depends on what you mean by "really needed". I am pretty sure the difference will be small, but considering how RF works, using an azimuth with a discontinuity at 0/360 is not the best you can do. It will treat events at, say, 359.9 and 0.1 as "maximally different" in that variable, while they are in reality the same. Remember we had spotted a strange pattern in the camera distribution of image centroids after gammaness cuts, when using the "psi" angle among the parameters. And this was because of psi also being an azimuth,