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

[ADAM-1512] Support VCFs with +Inf/-Inf float values. #1721

Merged
merged 2 commits into from Oct 9, 2017

Conversation

@fnothaft
Copy link
Member

@fnothaft fnothaft commented Sep 13, 2017

Resolves #1512. @heuermh do you have any VCFs that reproduce this issue? It would be good to add a test resource, if we've got one.

@fnothaft fnothaft added this to the 0.23.0 milestone Sep 13, 2017
@heuermh
Copy link
Member

@heuermh heuermh commented Sep 13, 2017

@heuermh do you have any VCFs that reproduce this issue? It would be good to add a test resource, if we've got one.

I do not, it was reported on gitter.
https://gitter.im/bigdatagenomics/adam?at=59b9a77ec101bc4e3ac33d56

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Sep 13, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2366/
Test PASSed.

@heuermh
Copy link
Member

@heuermh heuermh commented Sep 26, 2017

I hacked up sorted.vcf as a test file and am still seeing

2017-09-26 12:01:01 WARN  VariantContextConverter:361 - Caught exception
java.lang.NumberFormatException: For input string: "-Inf" when converting
[VC Unknown @ 1:14397-14400 Q. of type=INDEL alleles=[CTGT*, C]
attr={AC=2, AF=+Inf, AN=6, BaseQRankSum=-Inf, ClippingRankSum=0.138,
DP=69, FS=7.786, MLEAC=2, MLEAF=0.333, MQ=26.84, MQ0=0, MQRankSum=-1.906,
QD=1.55, ReadPosRankSum=0.384} GT=[[NA12878 CTGT*/C GQ 99 DP 20 AD 16,4
PL 120,0,827 FT rd {MQ=-Inf, PQ=+Inf}],[NA12891 CTGT*/C GQ 60 DP 10 AD 8,2
PL 60,0,414 FT dp;rd],[NA12892 CTGT*/CTGT* GQ 99 DP 39 AD 39,0
PL 0,116,2114]].
- support VCFs with +Inf/-Inf float values *** FAILED ***
...
@heuermh
Copy link
Member

@heuermh heuermh commented Sep 29, 2017

Created pull request fnothaft#20 with additional fixes and test file.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Sep 29, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2407/

Build result: FAILURE

[...truncated 15 lines...] > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1721/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains ca48ea5 # timeout=10Checking out Revision ca48ea5 (origin/pr/1721/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f ca48ea50be4fd7dc120e65416850b0236735dc9dFirst time build. Skipping changelog.Triggering ADAM-prb ? 2.6.2,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.0,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.0,centosADAM-prb ? 2.6.2,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.0,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1512-inf-float branch from e20c32c to a703f18 Oct 8, 2017
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Oct 8, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2411/
Test PASSed.

@heuermh
Copy link
Member

@heuermh heuermh commented Oct 9, 2017

Did I mess up with git or maybe is github confused? What happened to commit e20c32c?

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Oct 9, 2017

@heuermh no, I think that was my screwup. Let me fix that up...

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Oct 9, 2017

@heuermh fixed; sorry about that, I'd dropped it during the rebase.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Oct 9, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2415/

Build result: FAILURE

[...truncated 15 lines...] > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1721/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 3e5bf28878d2ce3b893a2e280688c0de54bcfd59 # timeout=10Checking out Revision 3e5bf28878d2ce3b893a2e280688c0de54bcfd59 (origin/pr/1721/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 3e5bf28878d2ce3b893a2e280688c0de54bcfd59First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.2,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.0,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.0,centosADAM-prb ? 2.6.2,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.0,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft fnothaft force-pushed the fnothaft:issues/1512-inf-float branch from 72a8ab0 to e585469 Oct 9, 2017
@heuermh
Copy link
Member

@heuermh heuermh commented Oct 9, 2017

Sorry, that commit needs vcs.toVariantRDD()vcs.toVariants()
and vcs.toGenotypeRDD()vcs.toGenotypes()

...
[INFO] /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.2/SCALAVER/2.10/SPARK_VERSION/1.6.3/label/centos/adam-core/src/test/scala:-1: info: compiling
[INFO] Compiling 88 source files to /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.2/SCALAVER/2.10/SPARK_VERSION/1.6.3/label/centos/adam-core/target/2.10.6/test-classes at 1507524292274
[ERROR] /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.2/SCALAVER/2.10/SPARK_VERSION/1.6.3/label/centos/adam-core/src/test/scala/org/bdgenomics/adam/rdd/variant/VariantContextRDDSuite.scala:167: error: value toVariantRDD is not a member of org.bdgenomics.adam.rdd.variant.VariantContextRDD
[ERROR]     val variant = vcs.toVariantRDD().rdd.filter(_.getStart == 14396L).first()
[ERROR]                       ^
[ERROR] /home/jenkins/workspace/ADAM-prb/HADOOP_VERSION/2.6.2/SCALAVER/2.10/SPARK_VERSION/1.6.3/label/centos/adam-core/src/test/scala/org/bdgenomics/adam/rdd/variant/VariantContextRDDSuite.scala:172: error: value toGenotypeRDD is not a member of org.bdgenomics.adam.rdd.variant.VariantContextRDD
[ERROR]     val genotype = vcs.toGenotypeRDD().rdd.filter(_.getVariant == variant).first()
[ERROR]                        ^
[ERROR] two errors found
@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Oct 9, 2017

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2416/

Build result: FAILURE

[...truncated 15 lines...] > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1721/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains be91557a24d734b1b83b244fd14fd31678a6c023 # timeout=10Checking out Revision be91557a24d734b1b83b244fd14fd31678a6c023 (origin/pr/1721/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f be91557a24d734b1b83b244fd14fd31678a6c023First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.2,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,1.6.3,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,1.6.3,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.0,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.0,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.0,centosADAM-prb ? 2.6.2,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,1.6.3,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,1.6.3,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.0,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.0,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Oct 9, 2017

@heuermh yeah, I'd just patched that after the last failure. Let's see what this new failure is.

@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Oct 9, 2017

Need to update a unit test; will get that in a sec.

@heuermh
Copy link
Member

@heuermh heuermh commented Oct 9, 2017

Yup that glob count one is hard to keep in sync when rebasing

@fnothaft fnothaft force-pushed the fnothaft:issues/1512-inf-float branch from e585469 to 47774ca Oct 9, 2017
@fnothaft
Copy link
Member Author

@fnothaft fnothaft commented Oct 9, 2017

Yup, should be updated now.

@AmplabJenkins
Copy link

@AmplabJenkins AmplabJenkins commented Oct 9, 2017

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2417/
Test PASSed.

@heuermh heuermh merged commit ca36dd6 into bigdatagenomics:master Oct 9, 2017
1 of 2 checks passed
1 of 2 checks passed
codacy/pr Not so good... This pull request quality could be better.
Details
default Merged build finished.
Details
@heuermh
Copy link
Member

@heuermh heuermh commented Oct 9, 2017

Merged as commit ca36dd6. Thank you, @fnothaft

@heuermh heuermh added this to Completed in Release 0.23.0 Jan 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.