search

gammapy / gamma-cat

An open data collection and source catalog for gamma-ray astronomy
https://gamma-cat.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
15 stars 17 forks source link

Biteau catalog new #129

Closed pdeiml closed 7 years ago

pdeiml commented 7 years ago

Hi Christoph,

please check the calculation of the energy and of the flux which are in line 58f and line 63f in Add_Biteau.py, respectively.

Current status: All data sets in Biteau's catalog are added in corresponding ecsv-files in /input/data//

TODO:

cdeil commented 7 years ago

@pdeiml - Some comments:

pdeiml commented 7 years ago

I checked the spectral fluxes of source 49 (https://gist.github.com/cdeil/2942d2f0241d7ff8d25db8008e0fda03) and Add_Biteau.py calculates it correctly. If you agree with everything up to know, I will work off the enumeration in my first comment.

cdeil commented 7 years ago

OK, great! Let me know if you have more questions or when it's ready for final review.

pdeiml commented 7 years ago

In principle this PR is finished.

"make.py checks" still produces some errors: 1) The ecsv-files which are generated from Biteau's catalog do have the meta-key "mjd" which is currently not allowed or not known. Should I add this key to https://github.com/gammapy/gamma-cat/blob/master/input/schemas/dataset_source_info.schema.yaml ?? 2) The new references are not listed in https://github.com/gammapy/gamma-cat/blob/master/input/gammacat/gamma_cat_dataset.yaml Should I add the references?

cdeil commented 7 years ago

The MJD meta key for the SED files should be added to the list here: https://github.com/gammapy/gamma-cat/blob/master/gammacat/sed.py#L47

Yes, if you list the new references in gamma_cat_dataset.yaml that would be good, that will mean they are used for the catalog.

pdeiml commented 7 years ago

Ready for merging

cdeil commented 7 years ago

There's an issue for input/data/2014/2014A%26A...563A..91A/tev-000016-sed.ecsv. There's two sets of points in one SED file.

cdeil commented 7 years ago

I've checked that input/data/2000/2000AIPC..515..113P/tev-000049-sed.ecsv here matches what I computed in https://gist.github.com/cdeil/2942d2f0241d7ff8d25db8008e0fda03 So I won't review the code here: spectral point computation should be OK.

As far as I can see, the only remaining issue / TODO here is to separate out the "Low" and "High" state of input/data/2014/2014A%26A...563A..91A/tev-000016-sed.ecsv into two SED files:

$ grep '563A..91A'  other_data_collections/2015ApJ...812...60B/BiteauWilliams2015_AllData_ASDC_v2016_12_20.ecsv 
49.1793 | 41.3248 | 4.8118e+25 | 0 | 5.88161e-12 | 1.63695e-12 | 1.63695e-12 | 55120 | 55242 |  | High | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 1.10986e+26 | 0 | 6.61594e-12 | 1.17467e-12 | 1.17467e-12 | 55120 | 55242 |  | High | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 2.56307e+26 | 0 | 6.21071e-12 | 1.26734e-12 | 1.26734e-12 | 55120 | 55242 |  | High | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 5.92407e+26 | 0 | 7.55901e-12 | 1.77916e-12 | 1.77916e-12 | 55120 | 55242 |  | High | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 1.36834e+27 | 0 | 7.28581e-12 | 2.45768e-12 | 2.45768e-12 | 55120 | 55242 |  | High | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 4.8118e+25 | 0 | 8.12132e-13 | 4.73321e-13 | 4.73321e-13 | 55120 | 55242 |  | Low | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 1.10986e+26 | 0 | 7.72985e-13 | 2.63963e-13 | 2.63963e-13 | 55120 | 55242 |  | Low | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 2.56307e+26 | 0 | 1.05492e-12 | 3.22237e-13 | 3.22237e-13 | 55120 | 55242 |  | Low | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 5.92407e+26 | 0 | 1.20213e-12 | 4.0584e-13 | 4.0584e-13 | 55120 | 55242 |  | Low | IC310 | MAGIC | 2014A&A...563A..91A
49.1793 | 41.3248 | 1.36834e+27 | 0 | 8.00413e-13 | 4.25861e-13 | 4.25861e-13 | 55120 | 55242 |  | Low | IC310 | MAGIC | 2014A&A...563A..91A

@pdeiml - Maybe you can check if this is a scripting bug that also affects other datasets.

pdeiml commented 7 years ago

You are right. In Biteau's catalog this SED is divided in two regimes "High" and "Low" but the measurement time is the same for both. How is this possible? They can not measure two different fluxes for the same energy and at the time time, can they?

cdeil commented 7 years ago

As discussed on Slack just now: splitting of SEDs should be first by "note" and then by "mjd". There are cases (e.g. 2002A&A...393...89A) where "note" isn't filled, i.e. the splitting by "mjd" has to happen there.

pdeiml commented 7 years ago

The script firstly checks for a difference in "note" and then in "mjd". Hence, every time when there is a change in "note" and every time when there is a change in "mjd" a new ecsv-file is created. This is like it is supposed to be, isn't it?

Additionally, the "note" is written in the ecsv-file as a comment like e.g. comments: 'This data was collected for 2015ApJ...812...60B and contributed to gamma-cat by Jonathan Biteau, magic data marked by "High " in the paper.' Is this fine?

Moreover, I reordered the commits of last week. Now, the creation of the ecsv-files is the last commit and all other changes which are necessary (like adding mid as a key to gammacat/sed.py or adding the references to gamma cat_dataset.yaml) are done before.

Now, it should be ready for merging :-)

pdeiml commented 7 years ago

Ready for merging :-)

cdeil commented 7 years ago

I've checked 2000AIPC..515..113P and 2014A%26A...563A..91A again, and unless I made a mistake everything is as it should be now.

@pdeiml - Thanks for this large addition to gamma-cat!!!

cdeil commented 7 years ago

I've updated the auto-generated files in the output folder: 04cbd4757f4ee67c815982bac461b1dde08c8d32 and f0dfddf504b4ca450cab3e6d6339173fba3c84b9

cdeil commented 7 years ago

And I've re-run the scripts in gamma-cat-status:

Not all of the changes in those diffs are due to this Biteau PR, but some are, so I thought I'd mention it here for reference.