cta-observatory / cta-lstchain

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

Request to add the "alpha" parameter as additional field in the DL1 and DL2 containers. #321

Closed dipierr closed 4 years ago

dipierr commented 4 years ago

It would be useful to access directly the "alpha" image parameter instead of calculating it in a separate step.

vuillaut commented 4 years ago

alpha as in source dependant analysis?

maxnoe commented 4 years ago

This is probably a broader discussion, but there should be no source dependent parameters in anything up to and including DL3.

Source dependent analysis in general is something that should be avoided in my opinion.

vuillaut commented 4 years ago

That's indeed a broader discussion that we already had (and still have) in LST. See PR #247

rlopezcoto commented 4 years ago

source-dependent analysis should be avoided in the case of stereo analysis, but for single-telescope analysis it is very useful. alpha can be calculated from the already existing parameters, but it will be easier if it is just included...

maxnoe commented 4 years ago

but for single-telescope analysis it is very useful.

From my experience with FACT in the last 5 years I can only whole heartedly disagree.

Source dependent analysis, especially when using machine learning models in the event reconstruction is much more complicated, error-prone and did not deliver better results for us.

Maybe for the current status of LST is different.

dipierr commented 4 years ago

This is probably a broader discussion, but there should be no source dependent parameters in anything up to and including DL3.

Source dependent analysis in general is something that should be avoided in my opinion.

I see your point. However for standard analysis we assume to know the position of the source, so this would allow to skip one step. Anyway as you said is a broader discussion.

rlopezcoto commented 4 years ago

yes, but #247 was about using source-dependent parameters for training/estimating parameters.

@MaxNoe: I agree that we can disagree 😄 Seriously, it may be that it is not the case anymore with newer machine learning models, but source-dependent analysis was more performant for MAGIC single-telescope in the past. In any case, this possibility should be given for people to test (#247) and it should not harm to add one additional parameter to the DL2 files

vuillaut commented 4 years ago

In any case, there is currently a PR for that.

rlopezcoto commented 4 years ago

but no alpha parameter is calculated and filled at the DL2 level, right?

SeiyaNozaki commented 4 years ago

Now alpha parameter is not calculated in #247. But I can easily install alpha to fill it in DL2, and actually this will also make me happy! Is it fine to install it?

rlopezcoto commented 4 years ago

in my opinion, it will be useful as it was requested here, but if anybody opposes to include source-dependent parameters in DL2 files, we can discuss it

DirkHoffmann commented 4 years ago

IMHO, you should check early with CTAO/DPPS, what they foresee in the data model.

maxnoe commented 4 years ago

The thing is, CTA does not forsee source dependent analysis at all I think.

So, If you want to try out source dependent analysis (or already doing it), I am in favor of adding these parameters to DL2, but into their own group source_dependent_parameters. Or even parameters_on and parameters_off_<N> with a group positions defining which on/off positions were chosen.

Like this, it is clear which parameters assume a source positions and which do not (so are safe to be used for a source independent analysis).

rlopezcoto commented 4 years ago

at the moment, since we are performing ON observations, the alpha will be calculated with respect to the center of the camera

maxnoe commented 4 years ago

Yes, at the moment.

But let's seperate source dependent and independent parameters from the start and get it right. What's the harm?

I think we can only gain from this. Looking at the if statements in #247 checking which columns are dependent and which not, it would be much clearer to just:

image_params = pd.read_hdf('dl2.hdf', 'parameters')
image_params_source = pd.read_hdf('dl2.hdf',  'parameters_on')

And the danger of including source dependent parameters by accident in a source independent analysis is dramatically reduced.

Also like this, the source independent part could follow the CTA data model, and we only add another group of extra data.

rlopezcoto commented 4 years ago

sure, @SeiyaNozaki could you please add that to #247?

mdpunch commented 4 years ago

Hi all, As the inventor of "alpha", I would just say that there is probably no need to calculate it and to keep it in the DL1/2, especially since it is dependent on the assumed source position (which can change if searching for a source, or if the bending corrections are revised). If you have the centre-of-mass of the Hillas ellipse (x_cm,y_cm) and the angle of the ellipse major axis, then it is quite easy to calculate the alpha_source for a given source position (x_source, y_source). So, as long as those three variables are kept you should only need a well-tested helper function to give alpha for each event, which just has three subtractions, a couple of ifs, and a call to atan2. Good Luck, Michael.

moralejo commented 4 years ago

Hi,

alpha itself is indeed easily obtained, but at DL2 we will also have higher-level source-dependent parameters including energy and gammaness. A separate container (or containers) for source-dep things is the best solution. And at that point, no matter how easy the calculation of Alpha is, why not include it?

We are aware that source-dependent analysis is not part of the CTA requirements. But unless CTA is like one of those countries where whatever is not compulsory is forbidden, we can define these additional data containers to help us with the commissioning. I for one will be fine with trashing them as soon as LST becomes a stereoscopic system.

mdpunch commented 4 years ago

Hi Abelardo, Okay, I understand then why you want to include alpha for a default source position, since it goes with other variables. Still would be good to have (as well) a standard routine for calculating alpha for another given position!

maxnoe commented 4 years ago

@mdpunch already exists in ctapipe, or at least is a two liner with two other classical source dependent parameters (miss and disp):

https://github.com/cta-observatory/ctapipe/issues/1188#issuecomment-558213685

from ctapipe.image.hillas import camera_to_shower_coordinates
disp, miss = camera_to_shower_coordinates(source_x, source_y, cog_x, cog_y, psi)

# maybe use `arctan(miss / disp)` to automatically get the smaller angle
alpha = np.arctan2(miss, disp)
rlopezcoto commented 4 years ago

Added in #247