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

Improve config and reduce fulltest #163

Merged
merged 17 commits into from
Dec 6, 2024
Merged

Conversation

LouisLeNezet
Copy link
Collaborator

The truth variants set of NA12878 is now the one computed by Genome in a Bottle.

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/phaseimpute branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@LouisLeNezet LouisLeNezet self-assigned this Nov 26, 2024
@LouisLeNezet LouisLeNezet added the enhancement New feature or request label Nov 26, 2024
@@ -40,5 +41,5 @@ params {
phase = false

// Impute tools
tools = "glimpse1"
tools = "glimpse2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this change? I am confused as the reason behind this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, we are going to launch a completely different AWS mega test from the one before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The aim is to reduce the computational cost of the megatest as much as possible.
The glimpse2 software as less steps (no variant calling) and should also be more computationally efficient.
I'll add it to the changelog

@LouisLeNezet LouisLeNezet mentioned this pull request Dec 2, 2024
11 tasks
Copy link
Collaborator

@atrigila atrigila left a comment

Choose a reason for hiding this comment

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

While I may have some reservations about these changes, I also don’t want to block progress toward the release. I trust your verification that the full glimpse2 megatest works, that it is less computationally expensive, and aligns with the new genome in a bottle VCF :)

@LouisLeNezet
Copy link
Collaborator Author

LouisLeNezet commented Dec 5, 2024

I've checked on my cluster on a subset and everything worked perfectly ;)
After time verification glimpse2 run with 24% less time than glimpse1.

@LouisLeNezet LouisLeNezet merged commit 61c18f6 into nf-core:dev Dec 6, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants