Change request #1600
Read only necessary columns in GCTAEventList
| Status: | Closed | Start date: | 12/12/2015 | |
|---|---|---|---|---|
| Priority: | High | Due date: | ||
| Assigned To: |  Mayer Michael | % Done: | 0% | |
| Category: | - | |||
| Target version: | - | |||
| Duration: |
Description
Currently, in GCTAEventList::read(), we require a lot of FITS columns to be present which are not used in analyses at all, e.g. “CORE” or “XMAX”, etc. These values are also not accessible via getter functions from GCTAEventAtom.
The method GCTAEventList::read() should be simplified and only read from file what is actually needed and what is accessible via GCTAEventAtom. This boils down to the time, energy, direction, event_id, obs_id (and possibly phase).
The use case for this is that we want to be able to handle event lists that do not provide such optional columns.
Recurrence
No recurrence.
History
#1
Updated by Knödlseder Jürgen about 9 years ago
The reason why this is not done is that the class is also used to write an event list with all columns. The logic is that one should be able to read in and write out again a full event list without loosing information, even if the information is not used in GCTAEventAtom.
But I know that this is not very satisfactory and had in mind a more flexible method that can adjust to whatever is in the input file. Btw: I hope also that the CTA event list file format simplifies, hence that at the end we will have less columns.
#2
Updated by Mayer Michael about 9 years ago
Thanks for the feedback. We simply could split the reading and writing into two functions each:
read_required(): Read all necessary columns (e.g. energy, time, direction)read_optional(): Read all other columns that are present (similar as we do for the hillas parameters at the moment). We could then add a protected memberstd::vector<std::string> m_opt_columnsthat contains all optional column names that are present on loading.write_required(): Write all basic columns that are necessarywrite_optional(): Write all columns that were present on loading (i.e. contained inm_opt_columns).
Thus we wouldn’t need to touch GCTAEventAtom but still allow a consistent read()/write() scheme. What do you think?
#3
Updated by Knödlseder Jürgen about 9 years ago
I would propose the following:
- From
GCTAEventAtom, all parameters that are not used should be removed (i.e. we only keepm_index,m_dir,m_energy,m_time,m_event_id,m_obs_id, andm_phase. GCTAEventListreads all available columns, but only requiresEVENT_ID,TIME,RA,DEC,ENERGYand optionallyPHASE. It will fillGCTAEventAtomattributes for this information, and put all remaining columns in astd::vector<GFitsTableCol>.GCTAEventListwrites the mandatory information, and any optional information stored in thestd::vector<GFitsTableCol>member on output. In that way, whatever is read in is written out.
The protected members of the GCTAEventAtom would be
int m_index; //!< Index in list
GCTAInstDir m_dir; //!< Event direction
GEnergy m_energy; //!< Event energy
GTime m_time; //!< Event time
unsigned long m_event_id; //!< Event identifier
unsigned long m_obs_id; //!< Observation identifier
float m_phase; //!< Optional phase
The protected members of the GCTAEventList would be
GCTARoi m_roi; //!< Region of interest
std::vector<GCTAEventAtom> m_events; //!< Events
std::vector<GFitsTableCol*> m_columns; //!< Pointers to optional columns
bool m_has_phase; //!< Signal presence of phase
#4
Updated by Mayer Michael about 9 years ago
- Status changed from New to Pull request
- Assigned To set to Mayer Michael
Thanks for the input, I have added the respective changes on branch 1600-GCTAEventList-flexible-reading-writing
All unit tests run smoothly. In
All unit tests run smoothly. In
GCTAEventList, I have removed the following protected methods:
read_events_v0()read_events_v1()read_events_hillas()
All reading now happens in read_events(). I have also added a protected member m_has_detxy signalling if detector coordinates were available (they have to be assigned to GCTAEventAtom.m_dir if available. In GCTAEventList::init_members() I have set this boolean to true to induce ctobssim writing out those columns, too.
For GCTAEventAtom I removed all optional protected members. I also realised that actually m_obs_id is also not used in the code at all. Hence I also removed “OBS_ID” from required columns (as it was done in http://gamma-astro-data-formats.readthedocs.org/en/latest/events/index.html).
As soon the code is merged, I will try to deal with #1598.
Note that since a few days, when I modify the python interface, I need make clean and a full rebuild to make it run again. Otherwise I get the following error messages (don’t know if this is just a problem on my machine):
Post process module build/gammalib/_app.so error: install_name_tool: for: build/gammalib/_app.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_app.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_base.so error: install_name_tool: for: build/gammalib/_base.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_base.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_com.so error: install_name_tool: for: build/gammalib/_com.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_com.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_cta.so Post process module build/gammalib/_fits.so error: install_name_tool: for: build/gammalib/_fits.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_fits.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_lat.so error: install_name_tool: for: build/gammalib/_lat.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_lat.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_linalg.so error: install_name_tool: for: build/gammalib/_linalg.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_linalg.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_model.so error: install_name_tool: for: build/gammalib/_model.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_model.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_mwl.so error: install_name_tool: for: build/gammalib/_mwl.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_mwl.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_numerics.so error: install_name_tool: for: build/gammalib/_numerics.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_numerics.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_obs.so error: install_name_tool: for: build/gammalib/_obs.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_obs.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_opt.so error: install_name_tool: for: build/gammalib/_opt.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_opt.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_sky.so error: install_name_tool: for: build/gammalib/_sky.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_sky.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_support.so error: install_name_tool: for: build/gammalib/_support.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_support.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_test.so error: install_name_tool: for: build/gammalib/_test.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_test.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_vo.so error: install_name_tool: for: build/gammalib/_vo.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_vo.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_xml.so error: install_name_tool: for: build/gammalib/_xml.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_xml.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs Post process module build/gammalib/_xspec.so error: install_name_tool: for: build/gammalib/_xspec.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/lib" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/lib error: install_name_tool: for: build/gammalib/_xspec.so (for architecture x86_64) option "-add_rpath /Users/mimayer/Software/gammalib/src/.libs" would duplicate path, file already has LC_RPATH for: /Users/mimayer/Software/gammalib/src/.libs make[2]: *** [build] Error 1 make[1]: *** [install] Error 2 make: *** [install-recursive] Error 1
#5
Updated by Knödlseder Jürgen about 9 years ago
- Status changed from Pull request to Closed