VERITAS-Observatory / V2DL3

VERITAS (VEGAS and Eventdisplay) to DL3 Converter
BSD 3-Clause "New" or "Revised" License
8 stars 1 forks source link

VERITAS DL3 format review: Missing metadata #7

Closed TarekHC closed 6 years ago

TarekHC commented 6 years ago

Hi @tony32lin and @ralphbird

I will post here the small issues that still need to be fixed (most of them are trivial, but I prefer to add them here to have a decent implementation).

For the effective area: CREF5 = '(ENERG_LO:ENERG_HI,THETA_LO:THETA_HI)' For the energy migration: CREF7 = '(ETRUE_LO:ETRUE_HI,MIGRA_LO:MIGRA_HI,THETA_LO:THETA_HI)'

I will open now the issue in the joint-crab project to review VERITAS DL3 files, so other bugs might rise there, but for now it should be trivial to fix these. I will leave it to you as I'm not 100% sure I can reproduce the files without VEGAS installed...

I'll try to do a eventDisplay exporter these days... Let's see how far I get.

ralphbird commented 6 years ago

I based the fits file extensively on the MAGIC one, it is on the "todo" list to do a thorough check against the standard.

Also, I can confirm that DETX/DETY are agains the alt-az system. Does the direction matter? (i.e. which way is +ve?)

ralphbird commented 6 years ago

A few more bits picked up on by Christoph

We might want to make it an option to save the event type - in time I want to think about how we can save different EAs for the different event types and see what impact that has on spectral reconstruction.

tony32lin commented 6 years ago

@TarekHC I made the proper modification to the files. The regenerated DL3 files are on my branch. I will double check all the files against the standard again a bit later before putting in a pull request.

@ralphbird Saving EVENT_TYPE is now an option instead of the default behaviour.

TarekHC commented 6 years ago

Hi @tony32lin

If you did the work, you upload the files... I don't want to steal visibility! :D

I'm not sure if you tried to produce the plots of the joint-crab repo, but they are super easy to re-generate: download the repo, create a new conda environment with the "environment.yaml" file, replace the 2 VERITAS DL3 files in the data/veritas folder, and execute the following commands:

./make.py fit-spec --dataset veritas --tool gammapy
./make.py fit-spec --dataset joint --tool all
./make.py plot-crab
./make.py plot-fit
./make.py check-predicted-counts
./make.py butterfly-comparison --dataset veritas
./make.py butterfly-comparison --dataset joint
./make.py result-summary

Let me know if you prefer I take care of that part.

tony32lin commented 6 years ago

@TarekHC No problem I will do that :) Just thought you might want to double check before putting these out to wider audiences.

tony32lin commented 6 years ago

@TarekHC I've been double checking the fits files against the standard and found that neither the files from MAGIC nor HESS are fully in compliance with the standard. Both have some keyword missing. Is it the goal for the join-crab data release to have fits file that fully align with what the standard requires or just a workable version that is compatible with the analysis provided in the joint-crab ?

TarekHC commented 6 years ago

I've been double checking the fits files against the standard and found that neither the files from MAGIC nor HESS are fully in compliance with the standard. Both have some keyword missing.

Which keywords are missing? We should fix those changes, as the objective is indeed to comply with the specifications. Maybe you can list them here or directly put an issue in the joint-crab repo (probably better).

TarekHC commented 6 years ago

Just thought you might want to double check before putting these out to wider audiences.

I trust you more than I trust myself... :)

tony32lin commented 6 years ago

@TarekHC There's a couple of those in both HESS and MAGIC's file but mostly auxiliary information such as OBSERVER in the EVENT list. I will compile a list of the missing field for files from all three telescopes soon.

TarekHC commented 6 years ago

I agree actually. Let's add OBSERVER = The VERITAS Collaboration or something like that. I checked the CTA DC1 data and it includes something like "CTA Consortium"... So we could do something similar.