Skip to content

Conversation

@joverlee521
Copy link
Contributor

Description of proposed changes

Makes it so much easier to identify bottlenecks in workflows if they all just start out with benchmarks!

Related issue(s)

Prompted by nextstrain/ncov-ingest#446

Checklist

  • Checks pass

Makes it so much easier to identify bottlenecks in workflows if they
all just start out with benchmarks!
@joverlee521 joverlee521 requested a review from a team June 17, 2024 23:41
@joverlee521
Copy link
Contributor Author

Read the Docs failure related to #209

@joverlee521 joverlee521 merged commit c924671 into master Jun 18, 2024
@joverlee521 joverlee521 deleted the snakeamke-guide-benchmarks branch June 18, 2024 00:16
@corneliusroemer
Copy link
Member

corneliusroemer commented Sep 1, 2025

I personally dislike the benchmark directive as it adds noise and I never use it, one can just look at logs to see which steps take long - it'd be better if there was a snakemake option to enable benchmarks that didn't add 2 lines to each rule.

Prior discussion:

Related snakemake issues/PRs:

@joverlee521
Copy link
Contributor Author

Benchmarks are one of those things that are not used often, but are extremely helpful to have for optimizing workflows. It's nice when they are generated every run so that we have base stats for comparisons. They are especially useful since Snakemake got rid of the --stats flag. For example, benchmarks were used by @victorlin in ncov, @j23414 in norovirus/dengue, and most recently @kimandrews in tb work.

We can potentially remove this recommendation from the docs and remove the benchmark directives from workflows once the global benchmark option is available in Snakemake, but for now it seems more beneficial to keep if the only downside is adding 2 extra lines per rule.

@victorlin
Copy link
Member

+1 on what Jover said, but I get the frustration. It seems like most of the recommendations in our style guide exist to work around less-than-ideal default behavior of Snakemake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants