Skip to content
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

validatevariants -gvcf fix #3530

Merged
merged 3 commits into from
Aug 31, 2017
Merged

validatevariants -gvcf fix #3530

merged 3 commits into from
Aug 31, 2017

Conversation

jsotobroad
Copy link
Contributor

fix issue with validatevariants when validatign a gvcf and a record is followed by a record that is fully encompassed by the first one. The interval math was off in this case

jsotobroad added 2 commits August 28, 2017 17:24
…s followed by a record that the previous one fully encompasses. The interval math was off in this case
@codecov-io
Copy link

codecov-io commented Aug 28, 2017

Codecov Report

Merging #3530 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@              Coverage Diff               @@
##              master     #3530      +/-   ##
==============================================
+ Coverage     80.079%   80.109%   +0.03%     
- Complexity     17760     17791      +31     
==============================================
  Files           1188      1188              
  Lines          64410     64543     +133     
  Branches       10004     10022      +18     
==============================================
+ Hits           51579     51705     +126     
  Misses          8845      8845              
- Partials        3986      3993       +7
Impacted Files Coverage Δ Complexity Δ
...r/tools/walkers/variantutils/ValidateVariants.java 82.857% <100%> (+0.504%) 34 <0> (+2) ⬆️
...er/tools/spark/sv/discovery/AlignmentInterval.java 89.831% <0%> (-0.847%) 23% <0%> (-1%)
...nder/tools/spark/pipelines/ReadsPipelineSpark.java 92.857% <0%> (-0.246%) 16% <0%> (+8%)
...lotypecaller/readthreading/ReadThreadingGraph.java 88.101% <0%> (+0.253%) 142% <0%> (+1%) ⬆️
...institute/hellbender/utils/GenomeLocSortedSet.java 93.793% <0%> (+0.69%) 59% <0%> (ø) ⬇️
...institute/hellbender/tools/spark/bwa/BwaSpark.java 68.182% <0%> (+1.515%) 7% <0%> (+3%) ⬆️
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 76.316% <0%> (+1.974%) 38% <0%> (ø) ⬇️
...ute/hellbender/tools/spark/bwa/BwaSparkEngine.java 95.349% <0%> (+2.622%) 10% <0%> (+5%) ⬆️
...e/hellbender/engine/spark/SparkContextFactory.java 73.973% <0%> (+2.74%) 11% <0%> (ø) ⬇️
...stitute/hellbender/tools/HaplotypeCallerSpark.java 81.988% <0%> (+2.94%) 35% <0%> (+11%) ⬆️
... and 1 more

Copy link
Contributor

@yfarjoun yfarjoun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one possible optimization, otherwise 👍

@@ -205,10 +205,13 @@ public void apply(final VariantContext vc, final ReadsContext readsContext, fina
// if the current record is adjacent to the previous record and "overlap" them if they are so our set is as
// small as possible while still containing the same bases.
final int start = (previousInterval != null && previousInterval.overlapsWithMargin(refInterval, 1)) ?
previousInterval.getEnd() : refInterval.getStart();
genomeLocSortedSet.add(genomeLocSortedSet.getGenomeLocParser().createGenomeLoc(refInterval.getContig(), start, vc.getEnd()), true);
Math.min(previousInterval.getStart(), refInterval.getStart()) : refInterval.getStart();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this:

Math.min(previousInterval.getStart(), refInterval.getStart()) 

not always equal previousInterval.getStart() given the order of iteration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jsotobroad
Copy link
Contributor Author

@lbergelson fix for an incorrect assumption I had made previously, if you or someone else could review thanks!

@jsotobroad jsotobroad requested a review from lbergelson August 30, 2017 21:58
@jsotobroad jsotobroad merged commit a9c3b6c into master Aug 31, 2017
@jsotobroad jsotobroad deleted the js_validate_gvcf_error_fix branch August 31, 2017 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants