-
Notifications
You must be signed in to change notification settings - Fork 594
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
VS-849 - Use the annotation 'AS_MQ' for indels. #8261
VS-849 - Use the annotation 'AS_MQ' for indels. #8261
Conversation
Add monitoring script usage throughout VQSR Classic and Lite.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## ah_var_store #8261 +/- ##
================================================
Coverage ? 85.873%
Complexity ? 35517
================================================
Files ? 2194
Lines ? 167039
Branches ? 18005
================================================
Hits ? 143442
Misses ? 17216
Partials ? 6381 |
WDLs failing validation 😿 |
use_allele_specific_annotations = true, | ||
monitoring_script = "gs://gvs_quickstart_storage/cromwell_monitoring_script.sh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this going to work with the Cromwell Local backend test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean our integration test? I modified it to be a parameter to JointVcfFiltering.wdl so that that wdl's cromwell test will work. But now I realize that this might also break our integration tests?
-resource:hapmap,known=false,training=true,truth=true,prior=0 ~{hapmap_resource_vcf} \ | ||
-resource:omni,known=false,training=true,truth=true,prior=0 ~{omni_resource_vcf} \ | ||
-resource:1000G,known=false,training=true,truth=false,prior=0 ~{one_thousand_genomes_resource_vcf} \ | ||
-resource:dbsnp,known=true,training=false,truth=false,prior=0 ~{dbsnp_resource_vcf} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naive question: what do these priors mean here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's temporary - need to do this to use P&S best. I will make a comment - or ideally parameterize this change.
if [ -s ~{monitoring_script} ]; then | ||
bash ~{monitoring_script} > monitoring.log & | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this evaluates to false there won't be a monitoring.log
but the output is not optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup - thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to return a non-optional (zero length file) if no monitoring script is passed.
if [ -s ~{monitoring_script} ]; then | ||
bash ~{monitoring_script} > monitoring.log & | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Cleaned up and back to just the AS_MQ changes. |
Monitoring script removed.