fix: cache ref data #348
fix: cache ref data #348
Conversation
WalkthroughThe pull request modifies the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
workflow/rules/ref.smk (1)
Consider updating the vg/autoindex wrapper version
The rule is using wrapper version v5.3.0, but the latest available version in snakemake-wrappers is v0.40.0. Consider updating to benefit from potential improvements and bug fixes.
🔗 Analysis chain
Line range hint
201-201: LGTM! The addition of caching will prevent unnecessary recomputation of graph genome indices.The change correctly enables caching of the vg indices, which are computationally expensive to generate.
Let's verify the wrapper version is the latest:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there's a newer version of the vg/autoindex wrapper gh api graphql -f query=' { repository(owner: "snakemake", name: "snakemake-wrappers") { releases(last: 1) { nodes { tagName } } } }' | jq -r '.data.repository.releases.nodes[].tagName'Length of output: 226
Script:
#!/bin/bash # Find the wrapper version used in vg_autoindex rule rg -A 5 "rule vg_autoindex" workflow/rules/ref.smkLength of output: 219
Script:
#!/bin/bash # Find the wrapper version used in vg_autoindex rule with more context rg -A 15 "rule vg_autoindex" workflow/rules/ref.smkLength of output: 382
🧹 Nitpick comments (2)
workflow/rules/ref.smk (2)
184-184: Consider adding the index file to outputs.The changes correctly enable caching of the renamed VCF file. However, since this is a bgzipped VCF file (output is .vcf.gz), you might want to also include its index file in the outputs to ensure it's cached as well.
output: - "resources/{pangenome}.renamed.vcf.gz", + vcf="resources/{pangenome}.renamed.vcf.gz", + index="resources/{pangenome}.renamed.vcf.gz.tbi",Also applies to: 187-187
Line range hint
1-201: Overall: Changes effectively solve the caching issue.The changes form a complete solution to prevent unnecessary recreation of the pangenome by:
- Preserving all intermediate files by removing
temp()directives- Consistently applying
cache: "omit-software"across all rules- Ensuring the entire pipeline from VCF download to graph index creation is cached
This will significantly improve workflow efficiency by preventing redundant operations.
Consider documenting the caching behavior in the workflow documentation to help users understand:
- Which files are cached
- Where they are stored
- How to clear the cache if needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workflow/rules/ref.smk(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Quick_Tests
- GitHub Check: Long_Tests
🔇 Additional comments (2)
workflow/rules/ref.smk (2)
167-167: LGTM! The changes correctly enable caching of the contig mapping.The removal of
temp()directive and addition of caching will preserve the contig renaming results, which is appropriate for reference data processing.Also applies to: 172-172
153-153: LGTM! The changes correctly enable caching of the downloaded pangenome.The removal of
temp()directive and addition of caching will prevent unnecessary downloads of the pangenome VCF file.Let's verify the URL parameter is properly configured:
Also applies to: 158-158
✅ Verification successful
✓ Verified: Pangenome configuration is properly set up
The pangenome VCF URL is correctly configured to use HPP v1.1 resources, matching the required genome build (GRCh38) and species (homo_sapiens). Caching this file is appropriate.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the pangenome URL is accessible grep -A 1 'pangenome:' $(find . -name "config.yaml" -o -name "*.json") | grep 'vcf:'Length of output: 473
Script:
#!/bin/bash # Check main config file for pangenome settings echo "=== Main config file ===" cat ./config/config.yaml echo -e "\n=== Searching for pangenome patterns ===" rg -i "pangenome" -A 3 -B 1 --type yamlLength of output: 12288
Currently, the cached pangenom gets recreated on every new workflow. This is caused as file nececarry for creating the pangenome where not cached and therefore are downloaded/created on each new execution causing the pangenome to be recreated.
Summary by CodeRabbit