Improvements to Tool and Component functionality #1052

kosack · 2019-04-24T11:36:01Z

This PR is part of an effort to make some standard tools for DL1/DL2 production in ctapipe. One of the things I found lacking was the ability to see what the full configuration that was used in a Tool (i.e. the Tool's config attribute only gives you the options the user passed in, not the values used).

To make it possible to generate a fully populated config dictionary for a Component or Tool, this adds a few features:

Tools now have a get_current_config() method that returns the full config for an instance (this was only possibly for a Class with traitlets.config.Application). This is automatically registered with the Provenance system during Tool.run()
added an export_tool_to_commented_yaml function which takes a tool and its current config and writes out a YAML file with correct comments (it's a bit of a hack based on the traitlets.Application.write_config() code,, so perhaps not ready for production use, but it's a step in the direction of being able to read and write YAML configurations (so far only JSON and Python are supported for reading)
Added some nice HTML representations of Tools and Components for Jupyter notebooks
Tools must now "register" all sub-components so that their current configuration can be queried
the functionality in ctapipe.utils.tools has now moved to ctapipe.core.traits (both packages had similar scope)

e.g.:

class MyTool:
    ...
    def setup(self):
        self.mycomp = self.add_component(MyComponent(parent=self))

Then when one wants to get the full config, it looks like this:

tool = DisplayIntegrator()
...
pprint(tool.get_current_config())

{
         "DisplayIntegrator": {
            "channel": 0,
            "config_file": "",
            "event_index": 0,
            "extractor_product": "NeighborPeakWindowSum",
            "log_datefmt": "%Y-%m-%d %H:%M:%S",
            "log_format": "%(levelname)s: %(message)s [%(name)s.%(funcName)s]",
            "log_level": 10,
            "telescope": 458,
            "use_event_id": false
         },
         "SimTelEventSource": {
            "allowed_tels": [],
            "input_url": "/Users/kkosack/Data/CTA/Prod3b/gamma/gamma_20deg_0deg_run100___cta-prod3_desert-2150m-Paranal-merged.simtel.gz",
            "max_events": null,
            "skip_calibration_events": true
         },
         "EventSeeker": {},
         "NeighborPeakWindowSum": {
            "lwt": 0,
            "window_shift": 3,
            "window_width": 7
         },
         "HESSIOR1Calibrator": {},
         "CameraDL0Reducer": {},
         "CameraDL1Calibrator": {
            "clip_amplitude": null,
            "radius": null
         }
      }

In making this update, I ran into a lot of confusion and undocumented features of traitlets.config, so it might be better later to re-implement traitlets.config.Application altogether (I started a small notebook to explore that, seen here: https://gist.github.com/kosack/8158a2748a458bbece74a010abb02d3c but found that it is too much effort to get all the functionality now. Perhaps somebody else has the time)

- `get_current_config()` returns the *full* configuration of the instance, unlike the `.config` attribute which only returns the non-default settings. This includes even changes made during the running of the program, not from the input configuration files. - added a nicer HTML repr for notebooks -

kosack · 2019-04-24T11:44:15Z

For an example of the experimental YAML output:

print(export_tool_config_to_commented_yaml(tool))                                                 
#------------------------------------------------------------------------------
# DisplayIntegrator(Tool) configuration
#------------------------------------------------------------------------------
DisplayIntegrator:
    # Channel to view
    # Choices: any of [0, 1]
    # Default: 0
    channel: 0

    # name of a configuration file with parameters to load in addition
    # to command-line parameters
    # Default: ''
    config_file: ""

    # Event index to view.
    # Default: 0
    event_index: 0

    # ImageExtractor to use.
    # Choices: any of ['FullWaveformSum', 'FixedWindowSum',
    # 'GlobalPeakWindowSum', 'LocalPeakWindowSum',
    # 'NeighborPeakWindowSum',
    # 'BaselineSubtractedNeighborPeakWindowSum'] or None
    # Default: 'NeighborPeakWindowSum'
    extractor_product: "NeighborPeakWindowSum"

    # The date format used by logging formatters for %(asctime)s
    # Default: '%Y-%m-%d %H:%M:%S'
    log_datefmt: "%Y-%m-%d %H:%M:%S"

    # The Logging format template
    # Default: '%(levelname)s [%(name)s] (%(module)s/%(funcName)s):
    # %(message)s'
    log_format: "%(levelname)s [%(name)s] (%(module)s/%(funcName)s): %(message)s"

    # Set the log level by value or name.
    # Choices: any of (0, 10, 20, 30, 40, 50, 'DEBUG', 'INFO', 'WARN',
    # 'ERROR', 'CRITICAL')
    # Default: 30
    log_level: 20

    # Telescope to view. Set to None to display the firsttelescope
    # with data.
    # Default: None
    telescope: None

    # event_index will obtain an event using event_id instead of
    # index.
    # Default: False
    use_event_id: False

codecov · 2019-04-24T12:04:14Z

Codecov Report

Merging #1052 into master will increase coverage by 0.26%.
The diff coverage is 95.7%.

@@            Coverage Diff             @@
##           master    #1052      +/-   ##
==========================================
+ Coverage   83.44%   83.71%   +0.26%     
==========================================
  Files         181      179       -2     
  Lines       10439    10548     +109     
==========================================
+ Hits         8711     8830     +119     
+ Misses       1728     1718      -10

Impacted Files	Coverage Δ
ctapipe/core/plugins.py	`100% <ø> (ø)`	⬆️
ctapipe/core/container.py	`68.31% <ø> (ø)`	⬆️
ctapipe/io/simteleventsource.py	`98.46% <ø> (ø)`	⬆️
ctapipe/tools/plot_charge_resolution.py	`89.65% <100%> (-0.35%)`	⬇️
ctapipe/core/provenance.py	`93.1% <100%> (+0.18%)`	⬆️
ctapipe/analysis/camera/charge_resolution.py	`100% <100%> (ø)`	⬆️
ctapipe/tools/display_dl1.py	`90.09% <100%> (+0.09%)`	⬆️
ctapipe/core/traits.py	`97.22% <100%> (+1.57%)`	⬆️
ctapipe/io/eventsource.py	`98.11% <100%> (ø)`	⬆️
ctapipe/core/tests/test_tool.py	`100% <100%> (ø)`	⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fea8a57...d15e992. Read the comment docs.

- properly set the default log_format trait - allow Component *instances* to be registered (so we can get their config later) - added `get_current_config()`, similar to the one in Component, but returning the actual config for the Tool instance + all registered Component instances - add the full instance config to the provenance system - added an HTML repr for notebooks - fixed a type in test_component_current_config() - add tool example - always log the config (as info not debug) - ensure allowed_tels is a set internally - make sure set() is JSON serializable - show difference to default value in written config - added tests for new tool functionality - Update notebooks

…nto feature/improve-tool

into feature/improve-tool

…nto feature/improve-tool

thomasarmstrong

Just some minor comments for now

thomasarmstrong · 2019-05-14T06:43:23Z

ctapipe/core/tool.py

+        help='The Logging format template'
+    ).tag(config=True)
+
+    log_format = Unicode(


Is this duplicate intentional?

Nope! I'll fix it

thomasarmstrong · 2019-05-14T13:15:52Z

ctapipe/tools/display_dl1.py

-        True,
-        help='Display the photoelectron images on-screen as they '
-             'are produced.'
+        True, help="Display the photoelectron images on-screen as they " "are produced."


Extra "" or new line?

thomasarmstrong · 2019-05-14T13:17:49Z

ctapipe/tools/display_dl1.py

@@ -137,41 +142,31 @@ class DisplayDL1Calib(Tool):
    telescope = Int(
        None,
        allow_none=True,
-        help='Telescope to view. Set to None to display all '
-             'telescopes.'
+        help="Telescope to view. Set to None to display all " "telescopes.",


thomasarmstrong · 2019-05-14T13:18:25Z

ctapipe/tools/display_dl1.py

-                "Display the photoelectron images on-screen as they "
-                "are produced."
+                {"ImagePlotter": {"display": True}},
+                "Display the photoelectron images on-screen as they " "are produced.",


thanks- seems to be due to reformatting (strings like that get concatenated, but there's no need in this case)

watsonjj

Looks good

kosack added 4 commits April 23, 2019 18:20

added initial function to turn Component or Tool into yaml

f562ff1

show difference to default value in written config

f6a0074

replaced custom log code with LoggingConfigurable

8fbf381

kosack force-pushed the feature/improve-tool branch from 1f36b11 to 9e94133 Compare April 26, 2019 12:45

kosack added 3 commits April 26, 2019 14:49

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

7208e73

…nto feature/improve-tool

remove CameraR1Calibrator

519d24b

code style cleanups

c772203

kosack added the ready for review label Apr 26, 2019

kosack requested a review from watsonjj April 26, 2019 14:11

kosack mentioned this pull request May 2, 2019

DL1 Data Processing Tool #1066

Closed

17 tasks

kosack added 5 commits May 3, 2019 11:47

added docstrings

1a555f3

make main() func so

c1aed24

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

fe3bd38

…nto feature/improve-tool

fix to work with new CameraCalibrator

1b66d52

fixed muon tool

45d0617

kosack self-assigned this May 3, 2019

kosack and others added 10 commits May 7, 2019 13:43

Merge branch 'master' into feature/improve-tool

052dfdd

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

66cec24

…nto feature/improve-tool

Merge branch 'feature/improve-tool' of https://github.com/kosack/ctapipe

2c8ab62

into feature/improve-tool

removed superflous import

50d742c

removed utils/tools.py and moved to core/traits.py

6751111

fix some problems with the move of utils/tools.py

daf5a3e

fix imports borked during refactor

aa911aa

small changes to fix style complaints

e180e85

just some reformatting

542691d

Merge branch 'master' of https://github.com/cta-observatory/ctapipe i…

93dae3f

…nto feature/improve-tool

thomasarmstrong reviewed May 14, 2019

View reviewed changes

kosack added 3 commits May 15, 2019 11:21

fixed some strings that were not merged after reformatting

95404f1

removed duplicate log_format trait

5a7fae3

fixed typo

d15e992

thomasarmstrong approved these changes May 16, 2019

View reviewed changes

watsonjj approved these changes May 20, 2019

View reviewed changes

kosack merged commit ae9546d into cta-observatory:master May 20, 2019

kosack mentioned this pull request May 22, 2019

add support for YAML config files in core.Tool #1047

Closed

kosack deleted the feature/improve-tool branch October 2, 2020 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to Tool and Component functionality #1052

Improvements to Tool and Component functionality #1052

kosack commented Apr 24, 2019 •

edited

Loading

kosack commented Apr 24, 2019

codecov bot commented Apr 24, 2019 •

edited

Loading

thomasarmstrong left a comment

thomasarmstrong May 14, 2019

kosack May 15, 2019

thomasarmstrong May 14, 2019

thomasarmstrong May 14, 2019

thomasarmstrong May 14, 2019

kosack May 15, 2019

watsonjj left a comment

Improvements to Tool and Component functionality #1052

Improvements to Tool and Component functionality #1052

Conversation

kosack commented Apr 24, 2019 • edited Loading

kosack commented Apr 24, 2019

codecov bot commented Apr 24, 2019 • edited Loading

Codecov Report

thomasarmstrong left a comment

Choose a reason for hiding this comment

thomasarmstrong May 14, 2019

Choose a reason for hiding this comment

kosack May 15, 2019

Choose a reason for hiding this comment

thomasarmstrong May 14, 2019

Choose a reason for hiding this comment

thomasarmstrong May 14, 2019

Choose a reason for hiding this comment

thomasarmstrong May 14, 2019

Choose a reason for hiding this comment

kosack May 15, 2019

Choose a reason for hiding this comment

watsonjj left a comment

Choose a reason for hiding this comment

kosack commented Apr 24, 2019 •

edited

Loading

codecov bot commented Apr 24, 2019 •

edited

Loading