Change request #1600

Read only necessary columns in GCTAEventList

Added by Mayer Michael over 8 years ago. Updated over 8 years ago.

Status:ClosedStart date:12/12/2015
Priority:HighDue 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 over 8 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 over 8 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 member std::vector<std::string> m_opt_columns that contains all optional column names that are present on loading.
  • write_required(): Write all basic columns that are necessary
  • write_optional(): Write all columns that were present on loading (i.e. contained in m_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 over 8 years ago

I would propose the following:
  • From GCTAEventAtom, all parameters that are not used should be removed (i.e. we only keep m_index, m_dir, m_energy, m_time, m_event_id, m_obs_id, and m_phase.
  • GCTAEventList reads all available columns, but only requires EVENT_ID, TIME, RA, DEC, ENERGY and optionally PHASE. It will fill GCTAEventAtom attributes for this information, and put all remaining columns in a std::vector<GFitsTableCol>.
  • GCTAEventList writes the mandatory information, and any optional information stored in the std::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 over 8 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 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 over 8 years ago

  • Status changed from Pull request to Closed

Merged into devel.

What concerns you Python problem: I guess you are using El Capitan? There was an issue related to the fact that DYLD_LIBRARY_PATH cannot be used anymore, which I solved (#1563), but that fix created apparently a side effect. I opened a new issue for that (#1605).

Also available in: Atom PDF