pytroll / satpy

Python package for earth-observing satellite data processing
http://satpy.readthedocs.org/en/latest/
GNU General Public License v3.0
1.07k stars 295 forks source link

Extend documentation on custom compositors #714

Open gerritholl opened 5 years ago

gerritholl commented 5 years ago

Feature Request

Is your feature request related to a problem? Please describe.

I am trying to add a custom compositor and it is not trivial to understand how this should be implemented. The use of existing compositors is now well-documented (thanks @pnuu in #705) but for a developer wanting to add their own compositors documentation is still limited. Specifically, I'm trying to convert two fogpy compositors fogpy.composites.fls_day and fogpy.composites.fls_night, which are a few dozen lines each and interface to classes within the 1.5 kLOC fogpy.algorithms module. The fogpy compositors are built around the old mpop package and .cfg configuration files. From the fogpy documentation, I infer that those functions then became methods of an image attribute of an mpop scene.

Today, I think I need to code my own Compositor class, probably inheriting either GenericCompositor or CompositeBase, defining at least the __call__ method. I believe the two functions in the linked fogpy module will essentially translate to this method. I may also need to define __init__. From reading the source, I have the impression that the prerequisites field in the YAML-file are passed into the projectables argument of the __call__ method, but maybe some things get passed into __init__ as well.

My inference is mostly from reading the source and some conservations in the Pytroll slack.

Describe the solution you'd like

I would like that the developer guide part of the Satpy documentation grows a new section with a tutorial on "adding a custom compositor so Satpy". This should be closely related to the composites documentation added in #705, and ideally explain (either directly or by linking to existing or to-be-written documentation elsewhere):

This list may be incomplete.

Describe any changes to existing user workflow

Most users will be unaffected, but new developers needing to create new compositors will be helped.

The documentation will grow larger, which will mean there is more documentation to keep up to date in case things change. This will increase the price of API changes for Satpy developers and therefore make their life harder. Perhaps the increase in price of API changes will reduce their frequency, which will be a secondary benefit to users ;-)

Additional context

As I am trying to find out how to do it, I will write down my thought process, probably in comments to this issue. Some thoughts I have already written down in this initial comment. Those thoughts, as far as they are correct and perhaps even where they are not, as long as they are useful, should be useful to write a first draft of such compositor documentation.

gerritholl commented 5 years ago

What I have found so far (I might be wrong!):

The __call__ method for each subclass of GenericCompositor prepares up to four layers. A composite can be either in colour ("RGB") or black and white ("L"), and with or without an alpha layer. The __call__ method must calculate these layers, then call GenericCompositor.__call__. For example, in satpy.composites.cloud_products.PrecipCloudsRGB.__call__:

https://github.com/pytroll/satpy/blob/97e2474edd731fe4533de535eb2fd1b61ced602a/satpy/composites/cloud_products.py#L84-L121

So for GenericCompositor.__call__, the projectables refer to actual image channels (at most four), but for the subclass __call__ method (each individual compositor), the projectables can be arbitrary data: either satellite channels/bands, differences, angles, etc. I suppose this means it can also be of arbitrary length (the fogpy daytime fog compositor needs 7 satellite channels). The names of the inputs can be configured with a yaml file section such as this one:

https://github.com/pytroll/satpy/blob/97e2474edd731fe4533de535eb2fd1b61ced602a/satpy/etc/composites/visir.yaml#L272-L279

These inputs are then provided to the __call__ method through the projectables list, apparently by position. I don't know if the names within the yaml file are of any importance.

If the compositor needs additional parameters, this should be passed on to the __init__ method of the compositor:

https://github.com/pytroll/satpy/blob/97e2474edd731fe4533de535eb2fd1b61ced602a/satpy/composites/__init__.py#L1133-L1150

I could not find any example of such additional parameters being passed on to a compositor in a yaml file, but I did find an example in the docstring of the satpy.composites.SunZenithCorrector modifier:

https://github.com/pytroll/satpy/blob/97e2474edd731fe4533de535eb2fd1b61ced602a/satpy/composites/__init__.py#L377-L411

presumably such arguments could be added in the same way for compositors.

I hope I'm right so far, correct me if I'm wrong :)

djhoese commented 5 years ago

I would say everything you've written is correct. A couple things I can clear up or I wanted to mention:

  1. Hard requirements for the composites are (prerequisites) are positional because they are all required. optional_prerequisites are currently also positional but this is not preferred and in the long term we'd like them to be by name (with backwards compatibility for positional, but deprecation warnings). See #161 for details.

  2. Right now there really isn't a use case for GenericCompositor for images with more than 4 bands (RGBA). Most common is 3 (RGB). There are other image modes/types that are defined in in the GenericCompositor and more could be added in the future, but in general these should be image modes not "I want this many dimensions" so that they can be saved as geotiff or PNG images. GenericCompositor currently does some nice things with setting up metadata and naming dimensions which is why it is the base class for a lot of compositors. That said, you don't need to use it as the base class and could do whatever you wanted.

gerritholl commented 5 years ago

Related to #702 , from trying to understand what happens under the hood when I do

ls.load(["overview"])
ls.show("overview")

I have the impression that the second one looks for an enhancement with the name "overview", and applies it prior to visualisation. So when the current composites documentation states to Don’t forget to add changes to the enhancement/generic.yaml file too, it means that when adding a composite with name foo, one should also add an enhancement with the name foo — is that correct?

djhoese commented 5 years ago

Yes, mostly. When you do .show under the hood Satpy is creating an XRImage object, enhancing it based on a configuration, and then saving it to a PIL image in memory which it then calls .show on.

The idea behind the enhancements configuration files is that it builds a "decision tree" of what enhancements to apply based on the best matching configuration. So you could have an enhancement for all "true colors" regardless of what sensor they are for. Or you could have that but also one section for true colors where sensor: viirs and that would apply only to viirs true colors. So for most new composites the enhancement needs to be configured because otherwise the default of a dynamic linear stretch is used which is both bad for performance and usually not what people want, but it works (if you have a better idea for default enhancement I'm all ears).

I've mentioned this in other places but in the long run I would also like to make it possible for people to specify color limits in the .attrs for RGBs or even single band (L) DataArrays. These color limits would provide the default linear limits since that is what most basic enhancements require and these values would be used by default during the enhancement stage. I'm sure others (@pytroll/core) would want to expand that to include gamma too (or possibly all enhancement functions, but that gets dangerous and suggests a complete redesign).

gerritholl commented 5 years ago

Some more "thinking aloud" notes:

The standard satpy/etc/enhancements/generic.yaml has, for example:

  overview_default:
    standard_name: overview
    operations:
    - name: inverse
      method: &inversefun !!python/name:satpy.enhancements.invert ''
      args:
      - [false, false, true]
    - name: stretch
      method: *stretchfun
      kwargs: {stretch: linear}
    - name: gamma
      method: &gammafun !!python/name:satpy.enhancements.gamma ''
      kwargs: {gamma: 1.6}

Following this example, I have created in my own $PPP_CONFIG_DIR/etc/enhancements/generic.yaml:

  fls_day_default:
    standard_name: fls_day
    operations:
    - name: stretch
      method: !!python/name:satpy.enhancements.stretch
      kwargs:
        stretch: crude
    - name: colorize
      method: !!python/name:satpy.enhancements.colorize
      kwargs:
        palettes:
          - {colors: [(0, 0, 0), (250, 200, 40)]}

yet when I run get_enhanced_image(ls["fls_day"]), where ls is a Scene object and ls["fls_day"] is a composite, then Enhancer.__init__ evidently does read my enhancements.yaml file, but EnhancementDecisionTree._find_name does not find it.

This probably happens because I had my Compositor yaml configuration defined as:

  fls_day:
    compositor: !!python/name:fogpy.composites.FogCompositorDay
    prerequisites:
      - VIS006
      - VIS008
      - IR_016
      - IR_039
      - IR_087
      - IR_108
      - IR_120
      - cmic_cot
      - cmic_lwp
      - cmic_reff
    path_dem: /media/nas/x21308/DEM/dem_eu_1km.tif

from which a standard_name attribute was missing. Changing this to:

  fls_day:
    compositor: !!python/name:fogpy.composites.FogCompositorDay
    prerequisites:
      - VIS006
      - VIS008
      - IR_016
      - IR_039
      - IR_087
      - IR_108
      - IR_120
      - cmic_cot
      - cmic_lwp
      - cmic_reff
    standard_name: fls_day
    path_dem: /media/nas/x21308/DEM/dem_eu_1km.tif

should resolve the mismatch.

Apparently (as I have understood from @djhoese in slack), Satpy will use the composite that matches the most attributes, specifically the attributes hardcoded in EnhancementDecisionTree.__init__:

https://github.com/pytroll/satpy/blob/1e9f483e79d69dcb880eab84b2635930674b6b49/satpy/writers/__init__.py#L907-L914

Indeed, with this update, Satpy manages to find my Enhancement (as evidences by a TypeError entirely unrelated to the matching ;-)

gerritholl commented 4 years ago

See also #133.