Containers for waveforms and charge extraction #29
Conversation
-some features for data management such as GRID access, local access, ELOG reading (work in progress)
-some features for data management such as GRID access, local access, ELOG reading (work in progress)
-try to vetorize Federica charge computation method
to perform wfs and charge extraction fits.fz file one by one -possibility to write file by file
allaws shape of camera (1855) and not number of used pixels in run
to avoid np.uint16 limit reached bug when computing charge histogram
computation np.int16 -> np.uint16
expected_pixels_id now -change histo HG and LG computation in charge.py to avoid broken pixels
instance -updates user script ggrolleron (load wfs+compute charge)
There was a problem hiding this comment.
Hi @guillaumegrolleron !
Many thanks for this very useful PR !
I have suggested a couple of minor changes, mainly to ease the use of these classes by other users, and to further avoid hardcoding the low/hig gain indices in some places.
|
|
||
| @staticmethod | ||
| def load_run(run_number : int,max_events : int = None, run_file = None) : | ||
| """Static method to load from $NECTARCAMDATA directory data for specified run with max_events |
There was a problem hiding this comment.
Is it documented somewhere that a $NECTARCAMDATA environment variable needs to be set ?
There was a problem hiding this comment.
I forgot to document that, is it fine if I explain in the README.md of nectarchain how to set this environment variable ? I'm not sure of the best policy
There was a problem hiding this comment.
For the time being, it may not be wise to document it at the module root level. I think for instance the dqm uses another environment variable for the same purpose. Or just leave it as it, with an explanation in the docstring.
There was a problem hiding this comment.
Ok I see, I will follow the dqm method
| import os | ||
|
|
||
| import logging | ||
| logging.basicConfig(format='%(asctime)s %(name)s %(levelname)s %(message)s',level=logging.INFO,filename = f"{os.environ.get('NECTARCHAIN_LOG')}/{Path(__file__).stem}_{os.getpid()}_load_wfs_charge.log") |
There was a problem hiding this comment.
Document that a $NECTARCHAIN_LOG environment variable is expected ?
There was a problem hiding this comment.
It is not a mandatory variable, but I defined it in the README
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| SPE_run_number = [2633,2634,3784] |
There was a problem hiding this comment.
I guess at some point such arguments could be passed by the user instead of being hardcoded.
There was a problem hiding this comment.
Indeed it is better, I did it
ctapipe_io_nectarcam
There was a problem hiding this comment.
Many thanks for the changes, @guillaumegrolleron !
Are there other modifications you would like to add, or could we merge ?
|
There are no more modifications, you can merge @jlenain ! Thanks for review |
-Implementation of containers to store waveforms from
ctapipeEventSourcein a numpy array mindset.-Charges can be extract from this
WaveformsContainerand store in aChargeContainer. Charge computation can be done with any ever implementedImageExctractorfromctapipeor with user defined extractors.-Charges and waveforms can be read and written in a fits format