Custom workflow config merging

[victorlin]

Background

Pathogen workflows are parameterized by a set of configuration values. The default set of parameters should work out of the box, and is typically stored in a YAML file such as phylogenetic/defaults/config.yaml. The parameters can be adjusted by:

1. Modifying the default configuration file directly
2. Overlaying additional YAML configuration files using Snakemake's --config, --configfile, or --configfiles options

Current status

The default configuration file is modified directly when running things locally. Config overlays are commonly used by automation and external users.

However, Snakemake's merging behavior for config overlays does not always align with our needs, such as in the case of subsampling.

Proposal

• What? Custom config merging code.

• Where? At the start of the Snakemake workflow.

• How?

  Options described by @tsibley in augur subsample: merging of external & default configs rsv#106 (comment):

  • What we're doing now: pick a single consistent merge approach that's Good Enough and apply it globally, even though it causes us problems like this one and others.
  • Customize the merge approach used depending on the config context, e.g. when merging subsampling configs, it may be more useful more of the time to override all sample definitions for a build. This is better for the "typical" use cases, but worse for the "unexpected" cases and harder to explain/document to boot.
  • Support explicitly denoting in the overlaying config what should happen at each merge level, with some default behaviour based on either approach above. This is more complicated, but provides an escape hatch when the default merging behaviour hurts more than it helps.

Additional notes

• It would be good to solve this before propagating augur subsample to more pathogen repos – that way, we can avoid the workaround of using custom_subsample in place of subsample.
• A config schema would improve error handling, but these efforts can happen independently.
• Config pre-processing can still be done later.

Progress

• Draft of external merging script: nextstrain/measles@ad89480
• Draft of within-Snakemake merging function: Add shared functions for custom config merge shared#63

[victorlin]

This was discussed in last week's dev chat (notes doc: Improving configuration of pathogen workflows). Some new ideas:

• Look into doing the merging inside Snakemake, similar to how we have chosen to do config pre-processing within Snakemake.
• Update the user config flow to start with an editable copy of the default config in the analysis directory and running Snakemake with --replace-workflow-config. This sidesteps the entire merging problem.
  • Cons:
    • Defaults might be overwhelmingly verbose. Pre-processing should help by allowing for more condensed user config.
    • Updates to defaults won't immediately make it to user configs.

[victorlin]

Look into doing the merging inside Snakemake

This is possible. Example:

def custom_merge():
    """
    Load config files using a shallow override merge.

    This approach differs from Snakemake's built-in merge logic: when an
    overlay contains a dict, it completely replaces the default dict rather
    than merging it.
    """
    global workflow
    merged = {}
    for config_file in reversed(workflow.configfiles):
        config_path = Path(config_file)
        if not config_path.is_absolute():
            config_path = Path(os.getcwd()) / config_path
        with config_path.open() as f:
            contents = yaml.safe_load(f)
        merged.update(contents)
    return merged

config = custom_merge()

[victorlin]

reversed(workflow.configfiles) is oversimplified and does not cover all use cases. Latest implementation is nextstrain/shared#63.

[jameshadfield]

This was discussed in last week's dev chat

In that meeting we discussed while it's technically possible (probably!) to do this, I didn't think we should. We discussed confusion for people experienced with snakemake as well as the need to arbitrarily choose (and document) the merging rules (or come up with some way of exposing the configuration of merging behaviour). I left that meeting thinking we wouldn't pursue this direction. What's changed?

---

Update the user config flow to start with an editable copy of the default config in the analysis directory and running Snakemake with --replace-workflow-config. This sidesteps the entire merging problem. ... [cons:] Defaults might be overwhelmingly verbose. Pre-processing should help by allowing for more condensed user config.

Just to expand on this, the config we would place into the analysis directory would be the default YAML (e.g. with glob syntax, wildcard placeholders etc). That can then be expanded into a verbose, small-multiples config & stored in the (e.g.) results directory. So the expansion to small multiples ("pre-processing") is independent of the ability for workflows to export their default config (to me!). Of these two, I'd implement the pre-processing first, and keep the other part as an interesting idea we are still thinking about.

[victorlin]

I left that meeting thinking we wouldn't pursue this direction. What's changed?

I left that meeting thinking that we shouldn't do custom config merging outside of Snakemake, and that there were two new ideas to explore without explicit priority of one over the other. I like the first idea because it easily removes the need for workarounds like custom_subsample in measles config, which I figured we'd better do early on before adding augur subsample to more workflows. I haven't explored the flow with --replace-workflow-config mostly because it was 2nd on the list, but also because it seems to require a bigger shift in how we invoke Snakemake workflows.

---

the expansion to small multiples ("pre-processing") is independent of the ability for workflows to export their default config (to me!)

It's technically independent, but I think we'd want the ability to export default config if --replace-workflow-config is in the flow. A prototype would be good to get a feel for this – I'll look into making one.

[victorlin]

One thing to note with --replace-workflow-config is that it only applies to config files provided via command line. That means it doesn't work if user config is pulled in via lines in the Snakefile, as done in many workflows:

configfile: os.path.join(workflow.basedir, "defaults/config.yaml")

if os.path.exists("config.yaml"):
    configfile: "config.yaml"

A solution would need to remove the if block and call snakemake with --configfile config.yaml. For nextstrain build, that could be given at the end of the command. For nextstrain run, that could be something like:

--- a/nextstrain/cli/command/run.py
+++ b/nextstrain/cli/command/run.py
@@ -288,6 +288,12 @@ def run(opts):
     print(f"Running the {opts.workflow!r} workflow for pathogen {pathogen}")
 
     # Set up Snakemake invocation.
+    if os.path.exists(os.path.join(opts.analysis_directory, "config.yaml")):
+        opts.default_exec_args += [
+            "--replace-workflow-config",
+            "--configfile", "config.yaml",
+        ]
+
     opts.default_exec_args += [
         # Useful to see what's going on; see also 08ffc925.
         "--printshellcmds",

[victorlin]

Custom config merging would significantly change config handling in pathogen repos. Even if there are benefits to adoption within a single repo, it's unlikely we'd roll this out across all repos. Let's leave things as-is.

The export/edit/--replace-workflow-config path may still be worth considering, but that's a separate solution.