Change request #3555
cssens test source position ignored
| Status: | Closed | Start date: | 02/26/2021 | |
|---|---|---|---|---|
| Priority: | High | Due date: | ||
| Assigned To: |  Sadeh Iftach | % Done: | 100% | |
| Category: | - | |||
| Target version: | 2.0.0 | |||
| Duration: |
Description
The position of the test source in cssens is arbitrarily set to {lon,lat} = {0,0} in
https://github.com/ctools/ctools/blob/84115ca3c9ba3d3e9e03bde4979c9807316a9dda/cscripts/cssens.py#L166
There is no way for the user to specify other values.
This is critical for any use of cssens in the presence of other sources.
The expected behaviour would be to extract the position from the source file.
Proposed change for illustration (also changing the default lpnt, bpnt values):
def _set_obs(self, emin, emax, lpnt=None, bpnt=None):
if lpnt is None and bpnt is None:
container = gammalib.GModels(self['inmodel'].filename())
src_model = container[self['srcname'].string()]
pntdir = gammalib.GSkyDir()
pntdir.radec_deg(src_model['RA'].value(), src_model['DEC'].value())
lpnt = pntdir.l_deg()
bpnt = pntdir.b_deg()
A cleaner implementation would get rid of lpnt, bpnt all together in this function, and use ra/dec instead.
Alternatively, one can add new position-reference arguments to cssens.
Recurrence
No recurrence.
History
#1
Updated by Tibaldo Luigi about 3 years ago
Hi Iftach,
thank you for reporting this. Your proposal makes a lot of sense. Would you be willing to implement this and make a pull request?
Best, Luigi
#2
Updated by Sadeh Iftach about 3 years ago
#3
Updated by Tibaldo Luigi almost 3 years ago
- Status changed from New to Pull request
Iftach could not set the status to pull request, he had only access to new and closed, so I did it for him.
#4
Updated by Knödlseder Jürgen almost 3 years ago
- Assigned To set to Sadeh Iftach
- Target version set to 2.0.0
- % Done changed from 0 to 90
Thanks Iftach. I changed your status from Reporter to Developer which allows to switch the status to Pull requests. Thanks for committing the code and welcome as a co-author of ctools!
I will now integrated the code.
#5
Updated by Knödlseder Jürgen almost 3 years ago
I refactored the code since the _set_obs() method is not called when an observation container is provided on construction to the script. Furthermore, the source position was not set in _set_obs() method if an observation definition XML file was provided on input.
I introduced a new method _set_source() that is called in the _get_parameters() method and that deals only with setting the test source name and position.
The _set_obs() method uses the method introduced by Iftach to extract the pointing direction from the test source position, provided that the test source actually has a position (which is for example not the case for a FITS map). The method offsets the pointing from the test source position according to the offset parameter.
Finally I updated also the cssens reference manual to explain how the setting of the test source position is done.
#6
Updated by Knödlseder Jürgen almost 3 years ago
- Status changed from Pull request to Closed
- % Done changed from 90 to 100
I merged the code into the devel branch.
#7
Updated by Knödlseder Jürgen almost 3 years ago
- Tracker changed from Bug to Change request