Template auspice config files for our core builds

[jameshadfield]

The auspice config files were almost duplicates of each other, resulting
in 14 very similar JSONs across the open & genbank profiles. This made
it harder than it should be to change config values (e.g. see
#916) and also made it all-too-
easy for things to fall out-of-sync.

The adds a custom rule which is only run for nextstrain core builds.
The rule is intended to be human readable and makes it clear which
are the values which differ across open/genbank builds as well as across
the different builds within those profiles. The vast majority of the
config is unchanged.

There is one change in content made here: The orderings of {region, country,
division} colorings are now in that order for both profiles; previously
the open profile had them reversed.

[jameshadfield]

This gave me a very cryptic error when running locally:

Invalid threads definition: entries have to be defined as RULE=THREADS pairs (with THREADS being a positive integer).
Unparseable value: '{tree:'.

The error went away if I used a shell block within make_auspice_config rather than a run block. This may be an issue on my end, and commenting out this line isn't a good solution, but I ran out of debugging ideas!

[tsibley]

Hmm. It seems worth solving this before merge to me, or do you think not?

Can you paste more of the error/logs and how you're invoking the workflow? I can't reproduce this locally so far…

[tsibley]

I mean, I can reproduce the exact message if I do very specifically this, but obviously this is nonsense:

-# set-threads: tree=4
+set-threads: "{tree:"

Trying something less nonsense like:

-# set-threads: tree=4
+set-threads:
+  tree: 4

gets me a different (and reasonablish) error: Unparseable value: "{'tree': 4}".

[jameshadfield]

Yeah for sure I'd like to solve this - that's why I flagged it in review 😉

$ snakemake --version
7.3.8

With the "set-threads: tree=4" commented out

$ snakemake --cores 1 --profile nextstrain_profiles/nextstrain-gisaid/ -pf results/global/auspice_config.json

...normal output and everything works as expected and creates the config JSON

with "set-threads: tree=4" in ./nextstrain_profiles/gisaid/config.yaml, i.e. the config file is identical to current master

$ snakemake --cores 1 --profile nextstrain_profiles/nextstrain-gisaid/ -pf results/global/auspice_config.json
Config file defaults/parameters.yaml is extended by additional config specified via the command line.
Building DAG of jobs...
Using shell: /usr/local/bin/bash
Provided cores: 1 (use --cores to define parallelism)
Rules claiming more threads will be scaled down.
Conda environments: ignored
Job stats:
job               count    min threads    max threads
--------------  -------  -------------  -------------
auspice_config        1              1              1
total                 1              1              1

Cannot send slack message as the config does not define a channel and/or token.
Select jobs to execute...

[Thu May  5 13:07:21 2022]
Job 0: Making a custom auspice config.
Reason: Forced execution

Invalid threads definition: entries have to be defined as RULE=THREADS pairs (with THREADS being a positive integer). Unparseable value: '{tree:'. 

Other points to help debugging:

• open behaves the same as GISAID for me
• setting tree: 4 gives me the same error you got, Unparseable value: "{'tree': 4}".
• I can run wokflows locally, but it's been a long time since I ran the core nextstrain builds (my hard drive wouldn't be big enough I don't think!). Which is to say I don't run any workflows which specify a set-threads in a profile config yaml.
• With the config file's set-threads commented out, this invocation also triggers the bug:
  snakemake --cores 1 --profile nextstrain_profiles/nextstrain-gisaid -pf results/global/auspice_config.json --set-threads tree=4
• I get the same error when setting threads for a different rule:

$ snakemake --cores 1 --profile nextstrain_profiles/nextstrain-gisaid -pf results/global/auspice_config.json --set-threads auspice_json=1

Invalid threads definition: entries have to be defined as RULE=THREADS pairs (with THREADS being a positive integer). Unparseable value: '{auspice_json:'.

[jameshadfield]

I've reinstated the set-threads setting to allow an AWS test run. It looks like it's working without issue. If that passes then I'm happy to go with it, as that'll be n=2 setups which can't reproduce the issue, but I would really like to know what's going on!

[tsibley]

Ah, this is a Snakemake version issue. The Conda environment defined by this repo gets a 6.x version of Snakemake and the Nextstrain Docker runtime has a 5.x version, neither of which exhibit the "Invalid threads definition" error. However, if I use 7.3.8, 7.4.0, or 7.5.0 then I can reproduce. The error goes away again with 7.6.0.

Looking at the code/issues finds these:

• snakemake/snakemake@0771062
• fix: issue 1615 - Switch formatting condition for dictionary snakemake/snakemake#1617
• Invalid threads definition: entries have to be defined as RULE=THREADS pairs (with THREADS being a positive integer). Unparseable value snakemake/snakemake#1622

[jameshadfield]

Ouch. Well, at least we've figured it out.

The Conda environment defined by this repo gets a 6.x version of Snakemake

I'm missing something... where is this environment? We have workflow/envs/nextstrain.yaml, but that doesn't specify snakemake (as it's intended to be run by snakemake itself). The ncov docs say to install via the nextstrain docs which don't pin versions at all: mamba create ... snakemake. Augur itself specifies "snakemake >=5.4.0" but only as a dev dependency.

[tsibley]

Ah, you're right. I was confused. workflow/envs/nextstrain.yaml does include Snakemake transitively and it gets 6.8.0 (not the latest), but the mamba create command in our install docs gets latest Snakemake (7.6.1).

[tsibley]

(There was something else going on yesterday where I tried created a new Conda env with just Snakemake and got an older version, but I now suspect that related to missing channels and/or whatever the default Python pin was.)

[tsibley]

Good to make this easier and more consistent.

This sort of thing shouts a little bit for something like CUE, Jsonnet, or YAML Template Engine (YTE) (which Snakemake integrates support for in newer versions). (With YTE, we'd render a YAML template and then convert that JSON for Augur.)

[jameshadfield]

Thanks for 👀 tom - much appreciated!

Force pushed to incorporate requested changes & rebase onto master.

This sort of thing shouts a little bit for something like CUE, Jsonnet, or YAML Template Engine (YTE) (which Snakemake integrates support for in newer versions). (With YTE, we'd render a YAML template and then convert that JSON for Augur.)

I see things a bit differently - I think everyone in the team is familiar with python, and I think the added rule is straightforward to read through. I'd hope everyone feels comfortable editing it. On the contrary, I think few (?) people would be familiar with the ins-and-outs of whatever templating language was picked. That's not to say we shouldn't use a templating language somewhere someday, but that the move will come with a learning burden duplicated over many people.

[tsibley]

I see things a bit differently…

Nod. I see where you're coming from, and I do think the current approach in this PR is just fine. I see this sort of thing as a tradeoff—and I definitely tend to skew more towards "it's worth learning X, at least a little"—but this case isn't at a tipping point for me.

[jameshadfield]

Test builds worked as expected & the snakemake 🐛 has been solved so I plan to merge this on Monday (NZ)