Make HaplotypeCallerSpark extend AssemblyRegionWalkerSpark #5386
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5386 +/- ##
==============================================
+ Coverage 86.985% 87.025% +0.04%
- Complexity 30423 30439 +16
==============================================
Files 1850 1852 +2
Lines 140944 140936 -8
Branches 15505 15508 +3
==============================================
+ Hits 122600 122650 +50
+ Misses 12705 12647 -58
Partials 5639 5639
|
| new FeatureContext(features, assemblyRegion.getExtendedSpan()))).iterator(); | ||
| AssemblyRegionEvaluator assemblyRegionEvaluator = supplierBroadcast.getValue().get(); // one AssemblyRegionEvaluator instance per Spark partition | ||
| final ReadsDownsampler readsDownsampler = assemblyRegionArgs.maxReadsPerAlignmentStart > 0 ? | ||
| new PositionalDownsampler(assemblyRegionArgs.maxReadsPerAlignmentStart, header) : null; |
There was a problem hiding this comment.
Does the downsampler want to be configurable at this stage?
There was a problem hiding this comment.
Not sure that's needed yet since the examples and HaplotypeCaller both use the PositionalDownsampler. There would be a bit of work to ensure the downsampler is serializable.
| new DownsampleableSparkReadShard( | ||
| new ShardBoundary(shardedRead.getInterval(), shardedRead.getPaddedInterval()), shardedRead, readsDownsampler))) | ||
| .map(shardedRead -> { | ||
| final Iterator<AssemblyRegion> assemblyRegionIter = new AssemblyRegionIterator( |
There was a problem hiding this comment.
A potential optimization we may want to look at for the haplotype caller is being able to generate these assembly regions and reconstruct the constituent up the reads later to save on holding an entire bam in an rdd at a time. That doesn't seem easy to do given the way this class is structured.
There was a problem hiding this comment.
I've got a branch based on this one that does exactly that :-)
|
|
||
| @Override | ||
| protected AssemblyRegionArgumentCollection getAssemblyRegionArgumentCollection() { | ||
| return new ExampleAssemblyRegionWalkerSpark.ExampleAssemblyRegionArgumentCollection(); |
There was a problem hiding this comment.
Why are these arguments getting pulled in from the example tool? Why can't this be its own argument collection?
There was a problem hiding this comment.
That's a mistake - I've fixed it now.
tomwhite
force-pushed
the
tw_spark_files_optimization_hc_as_spark_walker
branch
from
a1730f7 to
a7caab2
Compare
…ll allowing ReadsPipelineSpark to share common code.
tomwhite
force-pushed
the
tw_spark_files_optimization_hc_as_spark_walker
branch
from
a7caab2 to
9d1c3b5
Compare
|
Thanks for the review @jamesemery. I have tried these changes on a cluster with an exome-sized input, with no performance issues, so I'd be comfortable merging. As you say it will be a useful basis for the HaplotypeCallerSpark work we are doing. |
…titute#5386) * Factor out command line options for assembly region walkers (Spark) * Make HaplotypeCallerSpark extend AssemblyRegionWalkerSpark, while still allowing ReadsPipelineSpark to share common code.
This brings HaplotypeCallerSpark into closer alignment with the walker version.
cc @jonn-smith @jamesemery @droazen