Addressing OOM in CohortExtract #7245
Conversation
kcibul
changed the title
| @@ -78,6 +78,7 @@ | |||
| public static final List<String> FILTER_SET_SITE_FIELDS = Arrays.asList(FILTER_SET_NAME,LOCATION_FIELD_NAME,FILTERS); | |||
|
|
|||
| public static final List<String> COHORT_FIELDS = Arrays.asList(LOCATION_FIELD_NAME, SAMPLE_NAME_FIELD_NAME, STATE_FIELD_NAME, REF_ALLELE_FIELD_NAME, ALT_ALLELE_FIELD_NAME, CALL_GT, CALL_GQ, CALL_RGQ, QUALapprox, AS_QUALapprox, CALL_PL);//, AS_VarDP); | |||
| public static final List<String> COHORT_FIELDS_NO_PL = Arrays.asList(LOCATION_FIELD_NAME, SAMPLE_NAME_FIELD_NAME, STATE_FIELD_NAME, REF_ALLELE_FIELD_NAME, ALT_ALLELE_FIELD_NAME, CALL_GT, CALL_GQ, CALL_RGQ, QUALapprox, AS_QUALapprox);//, AS_VarDP); | |||
There was a problem hiding this comment.
we can probably get rid of the //, AS_VarDP if we are not using it (I think I had added that in there).
| @@ -119,7 +124,8 @@ public ExtractCohortEngine(final String projectID, | |||
| this.filterSetName = filterSetName; | |||
|
|
|||
| this.variantContextMerger = new ReferenceConfidenceVariantContextMerger(annotationEngine, vcfHeader); | |||
| this.gnarlyGenotyper = new GnarlyGenotyperEngine(false, 30, false, emitPLs, true); | |||
| this.disableGnarlyGenotyper = disableGnarlyGenotyper; | |||
| this.gnarlyGenotyper = disableGnarlyGenotyper?null:new GnarlyGenotyperEngine(false, 30, false, emitPLs, true); | |||
| final VariantContext mergedVC = variantContextMerger.merge( | ||
| unmergedCalls, | ||
| new SimpleInterval(contig, (int) start, (int) start), | ||
| refAllele.getBases()[0], | ||
| disableGnarlyGenotyper?true:false, | ||
| this.disableGnarlyGenotyper?true:false, |
There was a problem hiding this comment.
this syntax is a little confusing here. we could either just pass this.disableGnarlyGenotyper or if we wanted to be more explicit about what we are doing and why - we could declare a variable like:
boolean removeNonRefSymbolicAllele = this.disableGnarlyGenotyper;
and then pass the new parameter in the method call.
also we should update or remove the comment above.
There was a problem hiding this comment.
yeah I agree -- good feedback
| @@ -156,8 +156,8 @@ task ExtractTask { | |||
| # Runtime settings: | |||
| runtime { | |||
| docker: "us.gcr.io/broad-dsde-methods/broad-gatk-snapshots:varstore_d8a72b825eab2d979c8877448c0ca948fd9b34c7_change_to_hwe" | |||
| memory: "10 GB" | |||
| disks: "local-disk 100 HDD" | |||
| memory: "12 GB" | |||
There was a problem hiding this comment.
(just a nice-to-have) does it make sense to document our rationale for these choices somewhere? even a distinction between "this value was chosen deliberately" and "this is an inherited value and we haven't thought about this much" could be useful in future?
Addresses https://github.com/broadinstitute/dsp-spec-ops/issues/307
Compared results against baseline and saw no changes in GIAB results using ACMG cohort