Type change int -> long to prevent tranche novel variant count overflow #7864
Conversation
in VQSR scattered mode
Codecov Report
@@ Coverage Diff @@
## master #7864 +/- ##
===============================================
- Coverage 86.946% 86.933% -0.013%
- Complexity 36929 36954 +25
===============================================
Files 2219 2221 +2
Lines 173667 173799 +132
Branches 18755 18775 +20
===============================================
+ Hits 150996 151089 +93
- Misses 16057 16081 +24
- Partials 6614 6629 +15
|
|
Would it be difficult to write a test for this with |
| public Tranche(final String name, final double knownTiTv, final int numNovel, final double minVQSLod, final VariantRecalibratorArgumentCollection.Mode model, final double novelTiTv, final int accessibleTruthSites, final int numKnown, final int callsAtTruthSites) { | ||
| public Tranche(final String name, final double knownTiTv, final long numNovel, final double minVQSLod, | ||
| final VariantRecalibratorArgumentCollection.Mode model, final double novelTiTv, | ||
| final int accessibleTruthSites, final long numKnown, final int callsAtTruthSites) { |
There was a problem hiding this comment.
Did you check the upstream call sites for uses of int to store the number of known/novel sites? If there's an int anywhere in the call chain the overflow issues will likely persist...
There was a problem hiding this comment.
I found one in the input parsing and fixed it. (The number of truth sites values are also ints, but with the resources we use today they're way, way below Integer.MAX_VALUE and I don't expect that to ever change based on the size of the human genome.)
|
@mcovarr added two tests (and refactored a dataprovider, because I'm a good person) -- take another look? |
|
Closes #7859 |
in VQSR scattered mode