Skip to content

Refactor Pipeparts

James Kennington requested to merge feature-pipeparts-update into master

Motivation

The pipeparts subpackage has a lot of essential elements for building pipelines. These elements were sparsely documented and were all contained within an __init__ file. This MR preserves backwards compatibility while adding a more modern library structure (akin to numpy, pytorch, tensorflow, etc), documentation, and tests for each element within pipeparts.

Changes

Refactor

The elements defined in pipeparts are now grouped into thematically coherent modules (listed below). Using the modules as import namespaces can improve code readability and robustness, see here for an excellent explanation of using Python modules as namespaces.

  • encode: encoding and decoding, like flac
  • filters: filters like iir, fir, bandpass
  • mux: muxing and demuxing
  • pipedot: serializing pipelines to DOT format
  • pipetools: utilities for creating elements
  • plot: visual and plotting elements
  • sink: sink elements
  • source: source elements
  • transform: generic transformations
  • trigger: generating triggers

There is an additional new module gstlal.gsttools that contains gst-level utilities for creating elements, checking gst types, etc.

Backwards Compatibility

Every effort has been made to preserve backwards compatibility so that all imports using legacy import paths still work as expected. For example, both of the following code snippets still work. In fact, we have a unittest that ensures that legacy import paths still exist.

# Legacy style import (still works)
from gstlal.pipeparts import mkflacenc

mkflacenc(...)
# module-namespaced import
from gstlal.pipeparts import encode

encode.flac(...)

Documentation

Each element now has a complete docstring, with a reference to either a third-party documentation or the underlying implementation (for lal-specific elements). These docstrings are automatically built into the gstlal docs as part of the build process; until this is merged, you can inspect the latest build artifacts to see the rendered html.

Testing

Each element has a unittest that checks if the element can be created with appropriate inputs. Note, these tests do not presently run the pipelines. Implementing test coverage has revealed several categories of problems with custom elements, and the unittests have been marked using pytest.mark to keep track. These problems fall into the following categories:

  • Underlying implementation no longer exists
  • Underlying implementation exists but is not included in build (commented out in Makefile.am)
  • Implementation is broken (e.g. relied on features of gstreamer<1.0)

A full list of which elements fall into which category is included below.

Notes

  1. This MR preserves backwards compatibility in that the pipeparts.__init__ file uses import as statements to import any renamed or refactored elements as their legacy names

Elements with Problems

Any feedback on these elements is encouraged. ideally, we should remove them from pipeparts (if we do not intend to fix) or mark the elements themselves as deprecated (if we intend to fix in future). The unittests for each element have been marked appropriately using testtools.impl_deprecated or testtools.broken

Underlying Implementation not found

  • gstlal.pipeparts.transform.abs
  • gstlal.pipeparts.transform.pow

Underlying Implementation not included in build

  • gstlal.pipeparts.transform.itac
  • gstlal.pipeparts.transform.mean
  • gstlal.pipeparts.transform.trim
  • gstlal.pipeparts.trigger.blcbc_trigger_gen
  • gstlal.pipeparts.trigger.burst_trigger_gen
  • gstlal.pipeparts.trigger.trigger_gen

Broken Implementations

  • gstlal.pipeparts.encode.igwd_parse

Python Implementations (that don't work)

  • gstlal.pipeparts.plot.channelgram
  • gstlal.pipeparts.plot.histogram
  • gstlal.pipeparts.plot.spectrum
  • gstlal.pipeparts.source.fake_ligo
  • gstlal.pipeparts.source.fake_aligo
  • gstlal.pipeparts.source.fake_avirgo
  • gstlal.pipeparts.transform.lho_coherent_null
Edited by James Kennington

Merge request reports