-
Notifications
You must be signed in to change notification settings - Fork 309
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
[ADAM-353] Fixing issue with SAM/BAM/VCF header attachment when running distributed #354
[ADAM-353] Fixing issue with SAM/BAM/VCF header attachment when running distributed #354
Conversation
} | ||
protected val rgs = RecordGroupDictionary.fromSAMHeader(hdr) | ||
|
||
// recreate header when requested to get around header not being serializable, |
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.
Remove trailing comma?
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.
Ah, yes! Thanks @hammer!
I've added a commit here that fixes #351 as well; we weren't handing symbolic alleles correctly. I'm glad to move that into it's own PR if people would prefer, LMK. |
5afa75b
to
c588a34
Compare
c588a34
to
a6f04c4
Compare
Rebased on master. |
@@ -24,7 +24,7 @@ | |||
<spark.version>1.0.1</spark.version> | |||
<parquet.version>1.4.3</parquet.version> | |||
<!-- Edit the following line to configure the Hadoop (HDFS) version. --> | |||
<hadoop.version>2.2.0</hadoop.version> | |||
<hadoop.version>1.0.4</hadoop.version> |
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.
Was this intensional?
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.
*intentional
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.
Ah, sorry, did that for testing on EC2. I'll move it back.
a6f04c4
to
8672534
Compare
Rebased, and fixed hadoop version issue. |
assert(allele.isNonReference && !allele.isSymbolic) | ||
val variant = createADAMVariant(vc, Some(allele.getBaseString)) | ||
val genotypes = extractNonReferenceGenotypes(vc, variant, calling_annotations) | ||
assert(allele.isNonReference, |
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.
Did you mean to drop the check if the allele is not symbolic?
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.
Yes; this causes the problem in #351 with structural variants. I've added bigdatagenomics/bdg-formats#27 and #358 which are necessary to cleanly fix issue #351.
[ADAM-353] Fixing issue with SAM/BAM/VCF header attachment when running distributed
Thanks, Frank! |
Fixes issue #353. Runs header attachment per partition; not the cleanest fix, but it'll do. Modifies the SAMHeaderWritable class; without this modification, I was running into serialization issues.
@ryan-williams Can you give this a shot on your end? I've tried this on EC2 and it seems to work OK but would like to confirm that it works OK for you before merging it.