Change request #564
ctbin should read input FITS file data after the user enters all parameters
Status: | Rejected | Start date: | 10/13/2012 | |
---|---|---|---|---|
Priority: | High | Due date: | ||
Assigned To: | - | % Done: | 0% | |
Category: | - | |||
Target version: | 1.0.0 | |||
Duration: |
Description
I ran ctbin on a 3.7 GB events.fits input file generated with ctobssim. It prompts for the filename
Input event list or observation definition file [mucho_events.fits]
and when I hit enter it reads the 3.7 GB from disk (and makes me wait for a long time) before prompting for the next parameter:
Start value for first energy bin in TeV [0.1]
and all the other parameters.
I think all tools (I didn’t check the others) should first read all parameters, then process the data while I go for a coffee.
Would there be a problem changing ctbin
to work that way?
The “problem” is that ctbin::get_parameters
calls obs.load_unbinned(m_evfile);
before getting the remaining parameters:
void ctbin::get_parameters(void)
{
// If there are no observations in container then add a single CTA
// observation using the parameters from the parameter file
if (m_obs.size() == 0) {
// Get name of CTA events file
m_evfile = (*this)["evfile"].filename();
// Allocate CTA observation
GCTAObservation obs;
// Try first to open as FITS file
try {
// Load event list in CTA observation
obs.load_unbinned(m_evfile);
// Append CTA observation to container
m_obs.append(obs);
// Signal that no XML file should be used for storage
m_use_xml = false;
}
// ... otherwise try to open as XML file
catch (GException::fits_open_error &e) {
// Load observations from XML file
m_obs.load(m_evfile);
// Signal that XML file should be used for storage
m_use_xml = true;
}
// Use the xref and yref parameters for binning (otherwise the
// pointing direction(s) is/are used)
//m_xref = (*this)["xref"].real();
//m_yref = (*this)["yref"].real();
} // endif: there was no observation in the container
// Get remaining parameters
m_emin = (*this)["emin"].real();
m_emax = (*this)["emax"].real();
m_enumbins = (*this)["enumbins"].integer();
m_proj = (*this)["proj"].string();
m_coordsys = (*this)["coordsys"].string();
m_xref = (*this)["xref"].real();
m_yref = (*this)["yref"].real();
m_binsz = (*this)["binsz"].real();
m_nxpix = (*this)["nxpix"].integer();
m_nypix = (*this)["nypix"].integer();
// Optionally read ahead parameters so that they get correctly
// dumped into the log file
if (m_read_ahead) {
m_outfile = (*this)["outfile"].filename();
m_prefix = (*this)["prefix"].string();
}
// Return
return;
}
Recurrence
No recurrence.
Related issues
History
#1 Updated by Knödlseder Jürgen about 10 years ago
- Priority changed from Normal to High
- Target version set to 1.0.0
We need to make sure that this is fixed for release version 1.0.0.
#2 Updated by Mayer Michael about 10 years ago
run()
-method. In my opinion, we could make the following change:
get_parameters()
just checks whether the file is existent (or sanity of other parameters)run()
actually loads the files before executing the analysis step
We might also think about including read
and load
methods to the ctools
base-class. read()
could read the parameters and checks for sanity, while load()
actually reads the input FITS files.
The only concern I have is that an error, which occurs on reading a file, might raise rather late, when Christoph already went for coffee
#3 Updated by Knödlseder Jürgen about 10 years ago
It’s in fact not easy to make a complete separation, at least for the moment.
Reading of user parameters is in general conditional to what the ctool actually knows already. For example, if the ctool has already an observation container, it won’t read observation data. Same for a model definition file. In some cases, a ctool needs to open a fie (or check at least it’s existence) to decide what to do next. So one needs to carefully check what can be done.
In the long run I would agree that we should have a section that reads the parameters and another that loads information, but we maybe should wait with introducing that until we understand better how to deal in a general way with the problem. I created an issue for that (#1350).
As a side effect, there is an annoying issue with the parameters that are dumped in the log file: they may be misleading as they are eventually not used, but the user does not know this. I create an action #1349 for that, but that action is only efficient until a clear separation is made (issue #1350).
So let’s keep this issue for fixing the specific ctbin problem.
#4 Updated by Knödlseder Jürgen over 9 years ago
- Status changed from New to Rejected
Decided that it’s not so important for end users. Would be nice, but a pain to implement.