Closed wschoenell closed 8 years ago
We (me and @tribeiro) have been using this code for a while without any problem. Will give 2 weeks more to people complain, then merge.
Can someone explain what problem this PR solves? I really don't get it.
The ChimeraObject
now haves __metadataOverrideMethod__
attribute which can be set externally to a chimera location i.e. /MyHeaderController/name
which contains an alternative getMetadata
method.
When you take an exposure, the camera in the process will ask the getProxy(__metadataOverrideMethod__).getMetadata(request)
instead the instrument's default getMetadata
method if __metadataOverrideMethod__
is not None
.
This is all to be able to set custom fits headers based on custom getMetadata
methods. This is one of the requirements to make the fits header files produced by T80 (and any other robotic telescope) to be compliant with the pipeline software.
An practical example of how the override process works can be seen here: https://github.com/astroufsc/chimera-headers/blob/ab708c92560cc8e6b6866f3e0da37f427db2664f/chimera_headers/controllers/headers.py#L86-L87
To be compliant with pipeline software we need headers, in any way, even manually if someone would like this task.
All you want is to get all headers from everything possibly running that affects that exposure, right!?
This Header instrument as it is now will cause huge performance issues by moving the getMetadata class to its own control. Every call to self.instrument.X is a remote call now instead of a local one if the responsible for it was the original instrument.
I think Site is the one to gather all info about what instruments compose an observatory or part of it, if you take an image using a camera, it asks the Header controller to get its headers, Header controller goes on the site containing that camera (it must be only unique) and iterate over all instruments listed there asking getMetadata on that instrument (which knows how to compute it locally). That seems sane in terms of performance and in terms of coupling.
PS: Site is weird and maybe a misnomer, in this context we can think of site as a group of instruments that make sense together.
On Thu, Aug 6, 2015, 21:17 Paulo Henrique Silva notifications@github.com wrote:
To be compliant with pipeline software we need headers, in any way, even manually if someone would like this task.
All you want is to get all headers from everything possibly running that affects that exposure, right!?
Yes and no. What we want is to have headers of everything but also to have
them on the standard which meets the observatory/pipeline software
standards. If, for example, the observatory standard is EXP-TIME
for
exposure time and the chimera standard is EXPTIME
, things will not work.
Of course this is a silly example. I can give you more realistic ones.
This Header instrument as it is now will cause huge performance issues by moving the getMetadata class to its own control. Every call to self.instrument.X is a remote call now instead of a local one if the responsible for it was the original instrument.
Maybe I did not got point but I don't see how this could interfere on the
performance of chimera. We have been using many remote calls all over the
code. Every call to self.instrument.getMetadata()
which is only for
metadata purposes will be a to
getManager().getProxy(self.__metadataOverrideMethod__).getMetadata(request)
if there is an metadata controller. This is the way that it is done now is
doing a remote call likewise the call over an controller... No?
I think Site is the one to gather all info about what instruments compose an observatory or part of it, if you take an image using a camera, it asks the Header controller to get its headers, Header controller goes on the site containing that camera (it must be only unique) and iterate over all instruments listed there asking getMetadata on that instrument (which knows how to compute it locally). That seems sane in terms of performance and in terms of coupling.
PS: Site is weird and maybe a misnomer, in this context we can think of site as a group of instruments that make sense together.
I don't really see how to do this without changing all the way how chimera gets headers from the instruments.
— Reply to this email directly or view it on GitHub https://github.com/astroufsc/chimera/pull/97#issuecomment-128478856.<img src=" https://ci4.googleusercontent.com/proxy/zHxB5hP5kJWclZO5wMlf67Px3befD42ccwXw_7rqLwyVE8vr7Nuatnv9u1jTJyF3nZJIwpa-soe9DczxwiDWI3ZYGZoQdGkrZFcruzg5PWbhSsBv9LtQSWlUEY2MdmnOEKTUB7mNsZoeI66Zra5xJU736AxcsQ=s0-d-e1-ft#https://github.com/notifications/beacon/AByXk27T3_Z0IkwI-rZo5J_AJfvv9WHOks5ok6pPgaJpZM4FiHmR.gif ">
hard to see what I wrote and what you answered...
Again, let's see if the parser gets it now.
On Thu, Aug 6, 2015 at 9:17 PM Paulo Henrique Silva < notifications@github.com> wrote:
To be compliant with pipeline software we need headers, in any way, even manually if someone would like this task.
All you want is to get all headers from everything possibly running that affects that exposure, right!?
Yes and no. What we want is to have headers of everything but also to have them on the standard which meets the observatory/pipeline software standards. If, for example, the observatory standard is
EXP-TIME
for exposure time and the chimera standard isEXPTIME
, things will not work. Of course this is a silly example. I can give you more realistic ones.This Header instrument as it is now will cause huge performance issues by moving the getMetadata class to its own control. Every call to self.instrument.X is a remote call now instead of a local one if the responsible for it was the original instrument.
Maybe I did not got point but I don't see how this could interfere on the performance of chimera. We have been using many remote calls all over the code. Every call to
self.instrument.getMetadata()
which is only for metadata purposes will be a togetManager().getProxy(self.__metadataOverrideMethod__).getMetadata(request)
if there is an metadata controller. This is the way that it is done now is doing a remote call likewise the call over an controller... No?
I think Site is the one to gather all info about what instruments compose an observatory or part of it, if you take an image using a camera, it asks the Header controller to get its headers, Header controller goes on the site containing that camera (it must be only unique) and iterate over all instruments listed there asking getMetadata on that instrument (which knows how to compute it locally). That seems sane in terms of performance and in terms of coupling.
I don't really see how to do this without changing all the way how chimera gets headers from the instruments.
PS: Site is weird and maybe a misnomer, in this context we can think of site as a group of instruments that make sense together.
— Reply to this email directly or view it on GitHub https://github.com/astroufsc/chimera/pull/97#issuecomment-128478856.
We have been using this since last year on T80 with no impact on the performance of the exposures. Merging until we don't have any more elegant way out to change the way chimera does header on images.
This PR fixes #50. It implements a way to have flexible headers for each observatory by the means of controller plugins. The changes needed on chimera code (and done here) are:
_saveImage()
to a new methodgetMetadata()
to be consistent with the other instruments structure.ChimeraObject
class named__metadataOverrideMethod__
. This variable, if different ofNone
, will store the location of the object which must contain agetMetadata()
method and will replace the instrument's (and site) originalgetMetadata()
method.ChimeraObject
:setMetadataMethod()
andgetMetadataOverride()
. The first one is used if one wants to override the default metadata method of a instrument by another object on chimera's namespace. The usage is something like:The second one should be called on all
getMetadata()
methods before the instrument's metadata itself being written. The call would be, for example:Other minor changes:
fakecamera.py
andsite.py
CHECKSUM
andDATASUM
header keywords to all files saved by chimera._saveImage()
could go away sometime, but now I left this way for compatibility with the existent camera drivers. Maybe when doing the wcs move to theImage
class (#93)An template of a controller that does modifications on a fits header can be seen here: https://www.github.com/astroufsc/chimera-headers
When running chimera with that controller configured for all the fake instruments, one haves a fits header like the below. All headers with
Custom.
in the beginning of the comment where changed by that controller. The others are static and cannot be changed.