Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snakefile: Set AUGUR_RECURSION_LIMIT=10000 #150

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

joverlee521
Copy link
Contributor

@joverlee521 joverlee521 commented Apr 13, 2023

Description of proposed changes

This change is a response to the recursion error that a user ran into when running the workflow with a large number of sequences during Nextstrain office hours on 2023-04-13.

Set the recursion limit globally instead of asking the user to remember to set the environment variable or edit their own Snakefile.

Testing

  • Checks pass

This change is a response to the recursion error that a user ran into
when running the workflow with a large number of sequences during
Nextstrain office hours on 2023-04-13.

Set the recursion limit globally instead of asking the user to remember
to set the environment variable or edit their own Snakefile.
@joverlee521 joverlee521 requested a review from a team April 13, 2023 21:37
@joverlee521
Copy link
Contributor Author

We've never seen this issue with the Nextstrain automated builds in AWS Batch because the nextstrain-job definition already sets the AUGUR_RECURSION_LIMIT to 10000.

Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Separately, we might also revisit adding to Augur a default bump of the recursion limit. I think the question in the past has been, "what would that be?". The answer might be 10,000 if that's a value we're broadly successful with (at least for now).

@joverlee521 joverlee521 merged commit 08e70f7 into master Apr 14, 2023
@joverlee521 joverlee521 deleted the add-augur-recursion-limit branch April 14, 2023 21:16
joverlee521 added a commit to nextstrain/augur that referenced this pull request Apr 14, 2023
We've had to use the `AUGUR_RECURSION_LIMIT` environment variable to
bump the recursion limit for SARS-CoV-2 builds¹, mpox builds², and
our `nextstrain-jobs` definition in AWS Batch. From a general search of
the envvar across GitHub³, 10,000 looks like the default that others use
as well (although this may be influenced by our discussions⁴).

¹ nextstrain/ncov#690
² nextstrain/mpox#150
³ https://cs.github.com/?scopeName=All+repos&scope=&q=AUGUR_RECURSION_LIMIT%3D#328
joverlee521 added a commit to nextstrain/augur that referenced this pull request Apr 17, 2023
We've had to use the `AUGUR_RECURSION_LIMIT` environment variable to
bump the recursion limit for SARS-CoV-2 builds¹, mpox builds², and
our `nextstrain-jobs` definition in AWS Batch. From a general search of
the envvar across GitHub³, 10,000 looks like the default that others use
as well (although this may be influenced by our discussions⁴).

¹ nextstrain/ncov#690
² nextstrain/mpox#150
³ https://cs.github.com/?scopeName=All+repos&scope=&q=AUGUR_RECURSION_LIMIT%3D#328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants