Updates of NectarCam and LSTCam real data eventsource class #812

FrancaCassol · 2018-10-25T12:29:58Z

Added the instrument information to the real data containers. The camera geometry files have been adde to the ctapipe-extra repository. They are:

LSTCam-001.camgeom.fits.gz = present geometry of the LST1 camera (001 indicate the version)
NectarCam-001.camgeom.fits.gz = present geometry of the NectarCam prototype (001 indicate the version)

The optics is the one of MC data.

…nto LST_reader

…STCameraContainer (Fields: LSTEventContainer and LSTServiceContainer)

….0.2

…le with version 1.0.2 of the protozfit library

…nto LST_reader

…run but spread in multiple fits files

….0.2)

maxnoe · 2018-11-23T09:18:45Z

This is because the files in ctapipe-extra have a bug as Carl mentioned before. the VERSION header key word does not match the filename. You should update these first I guess.

FrancaCassol · 2018-11-23T09:22:56Z

Sorry, which version header? I have a pending PR in ctapipe-extra with a new version of the camera geometry, do you mean that? I am indeed what that that PR is a accepted... or do you mean other, that I missed?

FrancaCassol · 2018-11-23T09:24:10Z

I mean: I am waiting (not "what") (this stupid automatic spelling)

maxnoe · 2018-11-23T09:25:35Z

the version in the camgeom file name is 001 but the value of the fits header keyword VERSION is 1, .

The current code assumes that both are the same.

FrancaCassol · 2018-11-23T09:32:02Z

Ok, I missed totally this comment of Karl (actually now the geometry is 2), where did he wrote it?

What should I use: 002 or 2? (for both indeed)

maxnoe · 2018-11-23T09:33:10Z

I have no strong optionin on that. For sorting the files leading zeros might be nice if you expect more than 9 versions.

FrancaCassol · 2018-11-23T09:38:31Z

Sorry, I am still not sure to understand where is the problem. Could you please indicate me the place where karl peak about that?
Thanks

maxnoe · 2018-11-23T09:45:11Z

The issue is #813

maxnoe · 2018-11-23T09:47:28Z

If I understood @kosack correctly, it is assumend that the first part of the filename and the CAM_ID are identical, so:

In [1]: from astropy.io import fits
In [2]: f = fits.open('LSTCam-001.camgeom.fits.gz')
In [3]: 'LSTCam-001.camgeom.fits.gz'.split('.')[0] 
Out[3]: 'LSTCam-001'
In [4]: f[1].header['CAM_ID'] 
Out[4]: 'LST1Cam

Is not supported at the moment.

maxnoe · 2018-11-23T09:52:53Z

See also this issue:
cta-observatory/ctapipe-extra#13

FrancaCassol · 2018-11-23T10:10:22Z

But I made a PR in ctapipe-extra, so solve this problem solved: https://github.com/cta-observatory/ctapipe-extra/pull/14#issue-231899562

However it seems to me that this PR is blocked because it doesn't find the NectarCAM data in ctapipe-extra:
E FileNotFoundError: Couldn't find resource: 'NectarCAM.Run0890.10events.fits.fz'

?

maxnoe · 2018-11-23T10:54:12Z

I think you need to wait until this is merged, a new version is tagged and then update the ctapipe-extra version in .travis.yml

…nto nectarCAM_reader

FrancaCassol · 2018-12-04T10:18:01Z

Dear all,
is there any chance to get (in a close future ;-) ) a new release of ctapipe-extra (v0.2.16) with the correct LST and NectarCAM camera geometries? This is blocking this pull request since several weeks.

Otherwise, if this is not the correct way to proceed, could you please suggest me how to go further?
Thanks for any hint

Cheers
Franca

…EventSourceROOT

…nt they don't work with the use of a config dictionary but only with the direct set of the "input_url" argument.

FrancaCassol · 2018-12-12T14:35:46Z

Dear all,
I didn't get any reply to my last comment.
LST1 starts to take data and this missing accepted PR is very annoying for a lot of people.

I retry:
As far I understand this PR is blocked by a missing new release of ctapipe-extra. Can somebody help to de-block the situation? (any proposed solution will be welcome)

Thanks very much

Cheers
Franca

codecov · 2018-12-12T15:46:14Z

Codecov Report

Merging #812 into master will increase coverage by 0.25%.
The diff coverage is 97.04%.

@@            Coverage Diff             @@
##           master     #812      +/-   ##
==========================================
+ Coverage   73.97%   74.22%   +0.25%     
==========================================
  Files         213      213              
  Lines       12286    12397     +111     
==========================================
+ Hits         9088     9202     +114     
+ Misses       3198     3195       -3

Impacted Files	Coverage Δ
ctapipe/io/containers.py	`100% <100%> (ø)`	⬆️
ctapipe/io/tests/test_nectarcameventsource.py	`100% <100%> (ø)`	⬆️
ctapipe/io/tests/test_lsteventsource.py	`100% <100%> (ø)`	⬆️
ctapipe/io/nectarcameventsource.py	`96.22% <95.34%> (-3.78%)`	⬇️
ctapipe/io/lsteventsource.py	`94.15% <97.26%> (+5.6%)`	⬆️
ctapipe/instrument/camera.py	`97.6% <0%> (+0.59%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fad39f0...4fbc058. Read the comment docs.

FrancaCassol · 2018-12-14T07:36:12Z

Hi, I wonder if this PR can be accepted like that or I need to make a new push in my fork (where I changed the three remained style errors). I see from the merging the ctapipe HEAD that a lot of things changed and I wonder if this action will make a mess...

FrancaCassol · 2018-12-14T07:38:14Z

In the case the PR can be accepted like that, please can you do it?

dneise

Let us merge this and deal with style issues in a dedicated PR

* master: (22 commits) Remove all hillas_paramters but version 5, fixes cta-observatory#866 (cta-observatory#904) Fix docstring of EventSeeker, fixes cta-observatory#768 (cta-observatory#914) Do not set autodownload, fixes doc build (cta-observatory#910) Import bokeh.palletes correctly, fixes cta-observatory#911 (cta-observatory#912) Fix SST-1M to be DC and not SC. Solves cta-observatory#905 (cta-observatory#908) Fix a few test warnings (cta-observatory#902) Updates of NectarCam and LSTCam real data eventsource class (cta-observatory#812) Implemented FACT image cleaning (cta-observatory#862) remove `config=None, tool=None` in some tests (cta-observatory#897) Remove flow submodule (got moved to its own package) (cta-observatory#893) Cleaning the ctapipe folder. this has been empty for 3 years. (cta-observatory#892) Additional metadata from pyhessio, similar to cta-observatory#655 (cta-observatory#895) add scikit-learn to install_requires (cta-observatory#877) Add .mailmap (cta-observatory#894) Fix subarray peek. No more warnings. (cta-observatory#841) SimTelEventSource using pyeventio (cta-observatory#864) Use sparse neighbor matrix (cta-observatory#826) Add test how it should be (cta-observatory#842) fix errordef > 0. (cta-observatory#839) Fix warning about already closed hessio file (cta-observatory#834) ... # Conflicts: # ctapipe/analysis/camera/chargeresolution.py # ctapipe/tools/extract_charge_resolution.py

FrancaCassol and others added 30 commits September 28, 2017 12:47

New directory tree for the atmosphere calibration code

f1b8096

Fix doc error

66346d1

Merge branch 'master' of https://github.com/cta-observatory/ctapipe

88ad9d3

Merge branch 'master' of https://github.com/cta-observatory/ctapipe

cff4c0b

Merge branch 'master' of https://github.com/cta-observatory/ctapipe

b126726

Merge branch 'master' of https://github.com/cta-observatory/ctapipe

ee945fb

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

828408c

…nto LST_reader

LST data reader code

90f77fb

Add LST reader files

599b72e

Complete LST reader

6fc6a8d

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

b4c6f6b

…nto LST_reader

Improved container organisation: introduced EVT and SVT concept for L…

d4386fe

…STCameraContainer (Fields: LSTEventContainer and LSTServiceContainer)

Minor corrections

31d35e1

Style changes

fcc2f12

Style changes

679fbf9

Updated request for protozfitsreader library from release 0.44.5 to 1…

136ae43

….0.2

Change of import from module SimpleFile to module File to be compatib…

e118351

…le with version 1.0.2 of the protozfit library

remove ctapipe/atmosphere and doc/atmosphere

b95fbcb

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

3f224df

…nto LST_reader

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

3ef7c65

…nto LST_reader

Added a check on the number of gains in the LST event.waveform

902a88b

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

27972b0

…nto LST_reader

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

59b5e03

…nto LST_reader

Added class MultiFiles in order to read events belonging to the same …

a1cb9d5

…run but spread in multiple fits files

Correct the protozfits version (for the moment the reader need only 1…

937d542

….0.2)

Added comments to the MultiFiles class

e507ebb

Ask for version protozfit 1.40 and minor code corrections

3326fb2

Minor changes for better coding standard

dbaa23c

Minor style changes

10a23a3

Minor changes

3f78339

FrancaCassol added 4 commits November 23, 2018 12:49

Moved to last version of ctapipe-extra: 0.2.15

bffaf5b

move back to ctapipe-extra version: 0.2.14

7e82995

Go back to ctapipe-extra release 0.2.15

fda3259

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

089968f

…nto nectarCAM_reader

FrancaCassol added 2 commits December 4, 2018 16:58

Added the possibility to use wild chards in the file name as in MAGIC…

ebd77b5

…EventSourceROOT

Corrected code related to wildchards in input file name: for the mome…

03519c1

…nt they don't work with the use of a config dictionary but only with the direct set of the "input_url" argument.

FrancaCassol mentioned this pull request Dec 11, 2018

Flat-field camera corrections #870

Closed

Dominik Neise added 2 commits December 12, 2018 15:47

Merge branch 'master' into nectarCAM_reader

7b5bf9e

install ctapipe-extra 0.2.16 using pip

4fbc058

maxnoe approved these changes Dec 14, 2018

View reviewed changes

dneise approved these changes Dec 14, 2018

View reviewed changes

dneise merged commit a1fc0eb into cta-observatory:master Dec 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates of NectarCam and LSTCam real data eventsource class #812

Updates of NectarCam and LSTCam real data eventsource class #812

FrancaCassol commented Oct 25, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

maxnoe commented Nov 23, 2018 •

edited

Loading

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Dec 4, 2018

FrancaCassol commented Dec 12, 2018

codecov bot commented Dec 12, 2018 •

edited

Loading

FrancaCassol commented Dec 14, 2018

FrancaCassol commented Dec 14, 2018

dneise left a comment

Updates of NectarCam and LSTCam real data eventsource class #812

Updates of NectarCam and LSTCam real data eventsource class #812

Conversation

FrancaCassol commented Oct 25, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

maxnoe commented Nov 23, 2018 • edited Loading

maxnoe commented Nov 23, 2018

FrancaCassol commented Nov 23, 2018

maxnoe commented Nov 23, 2018

FrancaCassol commented Dec 4, 2018

FrancaCassol commented Dec 12, 2018

codecov bot commented Dec 12, 2018 • edited Loading

Codecov Report

FrancaCassol commented Dec 14, 2018

FrancaCassol commented Dec 14, 2018

dneise left a comment

Choose a reason for hiding this comment

maxnoe commented Nov 23, 2018 •

edited

Loading

codecov bot commented Dec 12, 2018 •

edited

Loading