Add parameter to enable overriding tree args

[huddlej]

Description of proposed changes

Adds a configuration parameter to the workflow, override_default_args,
in the tree section that allows users to toggle the corresponding new
flag in augur tree on or off. Sets the default value to true, since
the custom tree builder arguments we define in defaults are redundant
with previously hardcoded defaults in augur tree. This change should fix
a related segmentation fault on some systems, but it also allows users
to define more flexible tree builder arguments that conflict with the
defaults (e.g., bootstrap mode in IQ-TREE).

Related issue(s)

Related to nextstrain/augur#780

Testing

• Tested by CI
• Tested with full Nextstrain builds

Release checklist

If this pull request introduces backward incompatible changes, complete the following steps for a new release of the workflow:

• Determine the version number for the new release by incrementing the most recent release (e.g., "v2" from "v1").
• Update docs/src/reference/change_log.md in this pull request to document these changes and the new version number.
• After merging, create a new GitHub release with the new version number as the tag and release title.

[joverlee521]

Bumping this PR since we've had people as about this in our discussion forum (1, 2).

[victorlin]

Assigning myself to fix the merge conflict with configuration.md, since I migrated that to workflow-config-file.rst.

EDIT: done

[huddlej]

Thank you, @joverlee521 for bumping and @victorlin for signing up to fix the merge conflict.

The main reason I hadn't merged this already was that this PR does make a potentially breaking change. It sets the override of tree args as the default behavior, but if people already have custom tree-builder args in their config files, Snakemake's deep merge of their config with the defaults will cause the default tree builder args to be overridden by the user's arguments.

As a specific example, the original defaults prior to this PR were:

tree:
  tree-builder-args: "'-ninit 10 -n 4'"

The default arguments for IQ-TREE in augur tree are:

-ninit 2 -n 2 -me 0.05

As a result, the config entry above produces an IQ-TREE argument like:

-ninit 2 -n 2 -me 0.05 -ninit 10 -n 4

The redundant arguments in that command can cause IQ-TREE to crash, for some operating systems and versions of IQ-TREE. We try to solve this problem with the new argument that allows overriding the default arguments.

The proposed defaults in this PR are:

tree:
  tree-builder-args: "'-ninit 10 -n 4 -me 0.05'"
  override_default_args: true

As a result, the config entry above produces an IQ-TREE argument like:

-ninit 10 -n 4 -me 0.05

It is possible that users have defined their own tree builder arguments, based on the workflow behavior prior to this PR. For example, a common pattern would be to tell IQ-TREE to collapse zero-length branches in the tree instead of forcing bifurcations:

tree:
  tree-builder-args: "'-czb'"

In the current workflow, this config produces the following IQ-TREE argument, where the first three arguments are augur tree defaults:

-ninit 2 -n 2 -me 0.05 -czb

In this PR's implementation, this config would get deep merged with our new defaults to look like so:

tree:
  tree-builder-args: "'-czb'"
  override_default_args: true

As a result of this deep merge, the resulting IQ-TREE command would be:

-czb

This command could produce quite different results than the command with the current workflow behavior above.

Given that our recommended tree builder arguments require overriding the default augur tree values, I think we need override_default_args to be true by default. It's frustrating that there isn't a clearer way to let users know about this change of behavior short of updating the change log and making a new release, though.

Another (less than ideal) solution would be to make override_default_args false by default, so everyone keeps the redundant IQ-TREE arguments we've had this whole time. If users experience a crash with IQ-TREE, then we can recommend that they enable the override in their config. This solution maintains the status quo, so it shouldn't break any workflows that aren't already broken, but it does put the onus of resolving a known bug on the users who experience the bug.

[victorlin]

The mix of - and _ in parameter names right next to each other seems a bit off. Looking at the rest of this file, _ is used for everything but tree-builder-args. It's probably beyond this PR to change that, so probably fine to use underscores for override_default_args here to follow broader convention.