Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consistency of MAGIC and MAGIC+LST1 scripts #44

Closed
jsitarek opened this issue Apr 20, 2022 · 6 comments · Fixed by #48
Closed

consistency of MAGIC and MAGIC+LST1 scripts #44

jsitarek opened this issue Apr 20, 2022 · 6 comments · Fixed by #48

Comments

@jsitarek
Copy link
Collaborator

The scripts for MAGIC-only analysis (starting from MARS Y MC files) in scripts/magic and the ones for LST1+MAGIC analysis in scripts/lst1_magic are very much different and produce also differieng outputs (the way how different telescopes are written, the keys in H5 files where the parameters are stored, etc.) we should aim at standarizing them - probably best to fix them to the LST-like style

@YoshikiOhtani
Copy link
Collaborator

I think at first we need to ask Alessio if the scripts under scripts/mars are still used or will be updated or not - as far as I remember the scripts have not yet been updated to be used in a ctapipe v0.12 environment.

@jsitarek
Copy link
Collaborator Author

very likely this is still the case. I tried to use one and had to modify the imports and some class names (see PR #43 )

@aleberti
Copy link
Collaborator

Regarding the script magic_calibrated_to_dl1.py, that one was updated to use ctapipe 0.12, but not updated after some refactoring recently done by Yoshiki. Indeed the changes in PR #43 are only in imports (and small ones in config file). I think that it would be easier to adapt the script lst1_magic/magic_calib_to_dl1.py to work also for MCs, like it is done by magic_calibrated_to_dl1.py.

probably best to fix them to the LST-like style

does lst-chain use more or less the std DL1 format from ctapipe? I guess they still have something custom. Btw I wrote magic_calibrated_to_dl1.py so that the std DL1 format from ctapipe is used.

I think at first we need to ask Alessio if the scripts under scripts/mars are still used or will be updated or not - as far as I remember the scripts have not yet been updated to be used in a ctapipe v0.12 environment.

the scripts in scripts/mars have another purpose, e.g. create the files needed to perform the image comparison starting from star/superstar files.

For the scripts/magic folder, I guess that since most things are now implemented in the MAGIC-LST pipeline, we do not need them anymore. Anyway, let me have a look and then I can open a PR for the cleanup.

@aleberti
Copy link
Collaborator

About DL1/DL2 format in lst-chain, this PR cta-observatory/cta-lstchain#972 was opened just few mins ago.

@aleberti
Copy link
Collaborator

And just now I see #45 , so it is going already in the direction described above.

@YoshikiOhtani
Copy link
Collaborator

YoshikiOhtani commented Apr 22, 2022

Hi @aleberti,

the scripts in scripts/mars have another purpose, e.g. create the files needed to perform the image comparison starting from star/superstar files.

Yes sorry, there is a typo in my comment, I meant scripts/magic, not scripts/mars. And I'm fine to change the combined analysis pipeline to use the std DL1 format if it is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants