Refactor MC DL0 to DL1 script

[YoshikiOhtani]

In the pull request #72 there was a confusion related to the event source which is created several times in the MC DL0 to DL1 script. This actually could make confusions again in the future when we adapt the script to the cta-process tool. Then by looking into the script I found that it is not absolutely necessary and could be more simplified. So with this pull request I refactored the code by creating the event source only once, and also with the following minor changes:

• use the CameraFrames stored in subarray.tel[tel_id].camera.geometry.frame instead of creating them inside the script
• modified the muon analysis modules so as not to keep the telescope names, because now in the standard analysis the name is not used
• modified the logger information display to improve the readability

I confirmed that this pull request does not change the output results so I think I can safely merge it to the master branch. But I will keep it here a few days and so please let me know if you have any further requests or comments, especially @aleberti, @jsitarek and @gabemery. Otherwise I will merge it at the beginning of next week. Thanks!

[gabemery]

@YoshikiOhtani I would like to keep the information about the type of telescope available in the muon file.
So I would keep a field for it even if optional (maybe as telescope_type instead of name).
I can do the change for this if you agree.

[YoshikiOhtani]

@gabemery yes sure, please push commits to this branch, sorry for that I changed it without consulting with you...!

[gabemery]

It is done. Now in your script the subarray.tel[tel_id].name is used to keep information about the telescope type in the muon file.

[YoshikiOhtani]

Thanks a lot for the commit @gabemery. Any other comments/suggestions are very welcome.

[aleberti]

The changes look fine to me!

[YoshikiOhtani]

I faced an issue that some of the files processed with this refactored script cannot be merged with the merge_hdf_files.py script. I found that it is because the data type of the leakage parameters are different between the files, and if the first processed event is MAGIC it is np.float32, and if LST-1 it is np.float64. Actually it comes from the difference of the type of the image variable, which is originally np.float32, but when processing LST events it is converted to np.float64, for example in the process of increasing NSBs. It seems this difference only propagates to the leakage parameters - the other parameters (Hillas and timing) are computed as np.float64 in their functions. A simple solution is to convert also the MAGIC image variable to np.float64 which should not make any issues, and I confirmed that the files processed with this solution can be merged. So let me push a new commit about this solution to this branch now.

EDIT: for more information, the data types of the DL1 table columns are defined based on the first output event, so if the leakage columns are defined as np.float64 by a LST event, the leakage parameters of next MAGIC events will be saved as np.float64. That's why we didn't see this problem in the past, since we processed the events telescope wise (first LST, next M1, and last M2).

[YoshikiOhtani]

Hi, I gradually come back to my work and now I pushed several new commits to this blanch. Let me summarize what I changed here.

First of all I modified the script based on comments by flake8 with max-line-length = 90. I actually used pylint with the rc file but found that it is too strict and seems not to be considered well even in ctapipe. It looks to be used for Codacy check, but most of pull requests were merged without satisfying it. And also I prefer to use flake8 because it is simply faster than pylint.
Next, I removed the lines about the muon analysis from this script, because there is another pull request #83 which creates a dedicated script for the analysis so the contents will be overlapped.
Last, after updating the logger info and doing some minor refactoring, I run isort and black to this script.

[aleberti]

So, do you need another review on this?

[YoshikiOhtani]

Hi @aleberti, if you have time, yes please! But please note that I just pushed a few last-minute commits for the DISP and off-axis angle calculations. If you are fine with these changes refactoring, I would also like to do the similar modifications to the other scripts.

[aleberti]

Hi @aleberti, if you have time, yes please! But please note that I just pushed a few last-minute commits for the DISP and off-axis angle calculations. If you are fine with these changes refactoring, I would also like to do the similar modifications to the other scripts.

well, lmk when you are done with the commits and I can check the new changes.

[aleberti]

btw, do not worry about the failed tests in the CI...

[YoshikiOhtani]

well, lmk when you are done with the commits and I can check the new changes.

I'm already done with this script/pull request so it is ready for your review. What I meant is that I will do the similar modifications to the other pull requests. Sorry for the confusion!

btw, do not worry about the failed tests in the CI...

Yes sure :-)

[aleberti]

OK, I checked the code. Changes with black/isort are kind of safe I would say, so nothing much to say. All the other changes look good, so green light from my side.

[YoshikiOhtani]

Thank you very much for your checks. Since I want to use the new function to the other script, let me merge this pull request now.