Bug #1510
ctselect does not transfer RoI to selected file in case no event passes cuts
| Status: | Closed | Start date: | 07/09/2015 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assigned To: |  Mayer Michael | % Done: | 100% | |
| Category: | - | |||
| Target version: | 1.0.0 | |||
| Duration: |
Description
I have multiple observations with various energy thresholds. They were already preselected with
ctselect usepnt=yes rad=2.5. Now, I want to run/test
csspec in binned mode, which first calls ctselect on the input observation file. Since my observations have different energy thresholds, the energy selection in a particular slice might result in some runs having no event left (because their threshold is higher than the slice maximum).Using
csspec in binned mode, this selected GObservations object is then passed to ctbin to create a count cube. However, it seems ctselect has removed the previously applied RoI definition from the event list in case no event passes the selection.The problem seems to be in
ctselect.cpp:886
// If ROI selection has been applied then set the event list ROI
if (m_select_roi) {
GCTAInstDir instdir;
instdir.dir().radec_deg(ra, dec);
list->roi(GCTARoi(instdir, rad));
}
Since in
csspec, we pass “UNDEFINED” as RoI parameters, m_select_roi is set to false and thus no RoI gets assigned to the new event list. This causes an exception in ctbin as an RoI is required.I see two options to solve this issue:
- Change
ctselectto transfer the old RoI to the new event list (even though it is empty) - Change
ctbin::fill_cube()to check for the number of events before filling the cube. In case no event is given, the function could be left without checking the RoI information
I guess I would vote for option 2, what do you think? This should also be solved before 1.0
Recurrence
No recurrence.
History
#1
Updated by Mayer Michael almost 9 years ago
This problem also transfers to ctexpcube etc which throw an exception if no RoI is found. So maybe option 1 might be better suited after all?
Or we change the exception to warning, stating that this GCTAObservation is not used.
#2
Updated by Mayer Michael almost 9 years ago
- Status changed from New to Pull request
- Assigned To set to Mayer Michael
- % Done changed from 0 to 100
After some thinking, I decided to go for option 1.
Now the ctselect::select_events checks for previously existing GCTARoI and GEbounds. In case no RoI selection is performed, the old RoI will be restored and assigned to the new event list. The same now holds for the energy boundaries.
The fix is available on 1510-ctselect-should-consider-old-RoI
#3
Updated by Knödlseder Jürgen almost 9 years ago
I see that this pull request includes also a change in csspec that seems not to be related to this issue:
psfcube["anumbins"] = 20
# Compute Npred value
Npred = 0.0
if not self.m_binned:
for observation in like.obs():
Npred += observation.npred(source)
I would be concerned of fixing the number of bins for the PSF to 20. This is probably not enough. Normally you’d like a coarser grid for the spatial parameters.
Can you clarify what should go in with this pull request?
#4
Updated by Mayer Michael almost 9 years ago
Apologize for this commit on this branch. Apparently, I mixed up some commits. Please ignore this, I made the changes for testing purposes and they somehow got pushed together with the other changes. In #1513, however, I added the user parameter anumbins to csspec, and also that the npred-value is only computed if we have are in unbinned mode.
#5
Updated by Knödlseder Jürgen almost 9 years ago
#6
Updated by Knödlseder Jürgen almost 9 years ago
- Status changed from Pull request to Closed
Merged into devel.