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

add `adam view` command, analogous to `samtools view` #451

Merged
merged 7 commits into from Nov 7, 2014

Conversation

Projects
None yet
5 participants
@ryan-williams
Member

ryan-williams commented Oct 31, 2014

Currently supports -f and -F flags, reading from and writing to BAM or ADAM.

Intended to build on top of #450. The last commit is the only addition; you can view it directly here.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 31, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit 367c948 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision da53924 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f da53924 > git rev-list faa5542 # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Oct 31, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit 367c948 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision da53924 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f da53924 > git rev-list faa5542 # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Oct 31, 2014

Member

fixed source-formatting errors that failed the build

Member

ryan-williams commented Oct 31, 2014

fixed source-formatting errors that failed the build

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 31, 2014

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

AmplabJenkins commented Oct 31, 2014

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

@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Nov 1, 2014

Member

Looking forward to having this code in ADAM. Nice feature, Ryan.

Member

massie commented Nov 1, 2014

Looking forward to having this code in ADAM. Nice feature, Ryan.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 2, 2014

Member

Looks good to me; one question though, would it make sense to pull the ReadFilter functionality out into adam-core? It seems like it would be a useful thing to have in org.bdgenomics.adam.rdd.read.AlignmentRecordRDDFunctions.

Member

fnothaft commented Nov 2, 2014

Looks good to me; one question though, would it make sense to pull the ReadFilter functionality out into adam-core? It seems like it would be a useful thing to have in org.bdgenomics.adam.rdd.read.AlignmentRecordRDDFunctions.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

yea, good call @fnothaft; as part of doing this filtering using a predicate instead of a projection, I'm going to move those per-field filters into the adam-core///predicates directory, which should satisfy both your and Matt's suggestions at once. Are there particular things in AlignmentRecordRDDFunctions that you'd like to use such Predicates today?

Member

ryan-williams commented Nov 3, 2014

yea, good call @fnothaft; as part of doing this filtering using a predicate instead of a projection, I'm going to move those per-field filters into the adam-core///predicates directory, which should satisfy both your and Matt's suggestions at once. Are there particular things in AlignmentRecordRDDFunctions that you'd like to use such Predicates today?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 3, 2014

Member

Excellent! Off of the top of my head, the big common one is filtering for only mapped reads. That being said, if you had a "predicate builder" or some such thing that would take a flag value and return the appropriate predicate, that'd be really cool. I would use that all the time.

Member

fnothaft commented Nov 3, 2014

Excellent! Off of the top of my head, the big common one is filtering for only mapped reads. That being said, if you had a "predicate builder" or some such thing that would take a flag value and return the appropriate predicate, that'd be really cool. I would use that all the time.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

cool, yea I was just adding an abstraction for that; I'll look around for some existing candidate uses of it as well and lyk when it's ready

Member

ryan-williams commented Nov 3, 2014

cool, yea I was just adding an abstraction for that; I'll look around for some existing candidate uses of it as well and lyk when it's ready

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 3, 2014

Member

@ryan-williams that sounds awesome!

Member

fnothaft commented Nov 3, 2014

@ryan-williams that sounds awesome!

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

I'm a little confused about the primary alignment and secondary alignment fields of AlignmentRecord. Are they redundant? It seems like secondaryAlignment is not set in SAMRecordConverter and not included in the AlignmentRecordField enum; is it defunct / unused / deprecated?

Member

ryan-williams commented Nov 3, 2014

I'm a little confused about the primary alignment and secondary alignment fields of AlignmentRecord. Are they redundant? It seems like secondaryAlignment is not set in SAMRecordConverter and not included in the AlignmentRecordField enum; is it defunct / unused / deprecated?

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

hmm looking through it more I think I answered my own questions; ADAM is exploding SAMRecord's "primary" and "supplemental" fields into its three fields.

Next question: supplementaryAlignment is not included in AlignmentRecordField, though it is converted to and from SAMRecords. Any reason for that?

Member

ryan-williams commented Nov 3, 2014

hmm looking through it more I think I answered my own questions; ADAM is exploding SAMRecord's "primary" and "supplemental" fields into its three fields.

Next question: supplementaryAlignment is not included in AlignmentRecordField, though it is converted to and from SAMRecords. Any reason for that?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 3, 2014

Member

Next question: supplementaryAlignment is not included in AlignmentRecordField, though it is converted to and from SAMRecords. Any reason for that?

I don't think it's intentional; IIRC, we used to just have secondary (which isn't correct; the supplemental/secondary distinction gets everyone confused), then we fixed it. We probably just forgot to fix the AlignmentRecordField section of things.

Member

fnothaft commented Nov 3, 2014

Next question: supplementaryAlignment is not included in AlignmentRecordField, though it is converted to and from SAMRecords. Any reason for that?

I don't think it's intentional; IIRC, we used to just have secondary (which isn't correct; the supplemental/secondary distinction gets everyone confused), then we fixed it. We probably just forgot to fix the AlignmentRecordField section of things.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

cool, I'll add supplementalAlignment to AlignmentRecordField enum; does it get associated to the actual Avro field just as long as its .toString() matches the avro field name, or is there some other plumbing that happens?

Member

ryan-williams commented Nov 3, 2014

cool, I'll add supplementalAlignment to AlignmentRecordField enum; does it get associated to the actual Avro field just as long as its .toString() matches the avro field name, or is there some other plumbing that happens?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 3, 2014

Member

It's been a while since I've looked at the code, but I do believe that's the case.

Member

fnothaft commented Nov 3, 2014

It's been a while since I've looked at the code, but I do believe that's the case.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

hrm, I am stymied because ParquetInputFormat needs to take the class of the filter and instantiate it via a no-argument constructor; I don't see a way to do this dynamically using the values supplied to the cmdline-args -f and -F.

I tried to do it on my wip-view branch here but get instantiation errors, maybe because the class FilterFlagBitsPredicate that is dynamically-declared inside View is not in scope / accessible by Parquet:

  org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 1.0 failed 1 times, most recent failure: Lost task 0.0 in stage 1.0 (TID 1, localhost): parquet.hadoop.BadConfigurationException: could not instantiate unbound record filter class
        parquet.hadoop.ParquetInputFormat.createRecordReader(ParquetInputFormat.java:133)
        org.apache.spark.rdd.NewHadoopRDD$$anon$1.<init>(NewHadoopRDD.scala:115)
        org.apache.spark.rdd.NewHadoopRDD.compute(NewHadoopRDD.scala:103)
        org.apache.spark.rdd.NewHadoopRDD.compute(NewHadoopRDD.scala:65)
        org.apache.spark.rdd.RDD.computeOrReadCheckpoint(RDD.scala:262)
        org.apache.spark.rdd.RDD.iterator(RDD.scala:229)
        org.apache.spark.rdd.MappedRDD.compute(MappedRDD.scala:31)

I don't have any great ideas for working around this atm, lmk if you do.

Member

ryan-williams commented Nov 3, 2014

hrm, I am stymied because ParquetInputFormat needs to take the class of the filter and instantiate it via a no-argument constructor; I don't see a way to do this dynamically using the values supplied to the cmdline-args -f and -F.

I tried to do it on my wip-view branch here but get instantiation errors, maybe because the class FilterFlagBitsPredicate that is dynamically-declared inside View is not in scope / accessible by Parquet:

  org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 1.0 failed 1 times, most recent failure: Lost task 0.0 in stage 1.0 (TID 1, localhost): parquet.hadoop.BadConfigurationException: could not instantiate unbound record filter class
        parquet.hadoop.ParquetInputFormat.createRecordReader(ParquetInputFormat.java:133)
        org.apache.spark.rdd.NewHadoopRDD$$anon$1.<init>(NewHadoopRDD.scala:115)
        org.apache.spark.rdd.NewHadoopRDD.compute(NewHadoopRDD.scala:103)
        org.apache.spark.rdd.NewHadoopRDD.compute(NewHadoopRDD.scala:65)
        org.apache.spark.rdd.RDD.computeOrReadCheckpoint(RDD.scala:262)
        org.apache.spark.rdd.RDD.iterator(RDD.scala:229)
        org.apache.spark.rdd.MappedRDD.compute(MappedRDD.scala:31)

I don't have any great ideas for working around this atm, lmk if you do.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

thought I had a way around this issue but was mistaken.

going to just go without projection or predicate for now.

Member

ryan-williams commented Nov 3, 2014

thought I had a way around this issue but was mistaken.

going to just go without projection or predicate for now.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 3, 2014

Member

alright, I've retooled adam view sans projection/predicate, and added a test (did you know that only 1000 of the 4096 possible values of the 12 flag-field bits are considered valid by HTSJDK's SAM-parsing?).

I'm going to test running this on my Spark cluster, but I've tested this a decent amount locally and it seems solid, so you guys can have another look.

Member

ryan-williams commented Nov 3, 2014

alright, I've retooled adam view sans projection/predicate, and added a test (did you know that only 1000 of the 4096 possible values of the 12 flag-field bits are considered valid by HTSJDK's SAM-parsing?).

I'm going to test running this on my Spark cluster, but I've tested this a decent amount locally and it seems solid, so you guys can have another look.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 3, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit e560f30 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision 3d58dd3 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 3d58dd3 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Nov 3, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit e560f30 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision 3d58dd3 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 3d58dd3 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 4, 2014

Member

it seems like the issue was that my loading of my flag-values.sam test file via ClassLoader.getSystemClassLoader.getResource("flag-values.sam").getFile was, when run as part of jenkins-test, looking for /home/jenkins/workspace/ADAM-prb/hadoop.version/2.3.0/label/centos/adam-core/target/adam-core-0.14.1-SNAPSHOT-tests.jar!/flag-values.sam, which doesn't exist.

I've added a somewhat clunky attempt to catch the FileNotFoundException and try again with ClassLoader.getSystemClassLoader.getResource("src/test/resources/flag-values.sam").getFile; I can't reproduce the failure locally so I'm going to see what Jenkins says this time before sinking too much more thought into it.

Member

ryan-williams commented Nov 4, 2014

it seems like the issue was that my loading of my flag-values.sam test file via ClassLoader.getSystemClassLoader.getResource("flag-values.sam").getFile was, when run as part of jenkins-test, looking for /home/jenkins/workspace/ADAM-prb/hadoop.version/2.3.0/label/centos/adam-core/target/adam-core-0.14.1-SNAPSHOT-tests.jar!/flag-values.sam, which doesn't exist.

I've added a somewhat clunky attempt to catch the FileNotFoundException and try again with ClassLoader.getSystemClassLoader.getResource("src/test/resources/flag-values.sam").getFile; I can't reproduce the failure locally so I'm going to see what Jenkins says this time before sinking too much more thought into it.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft
Member

fnothaft commented Nov 4, 2014

Ping @shaneknapp.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit ac43b72 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision c5d4591611db50bc1c6d41caa619e0f3c31aedc4 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f c5d4591611db50bc1c6d41caa619e0f3c31aedc4 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit ac43b72 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision c5d4591611db50bc1c6d41caa619e0f3c31aedc4 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f c5d4591611db50bc1c6d41caa619e0f3c31aedc4 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@shaneknapp

This comment has been minimized.

Show comment
Hide comment
@shaneknapp

shaneknapp Nov 4, 2014

Contributor

hmm, a quick look on that slave shows that the adam-core-0.14.1-SNAPSHOT-tests.jar is indeed there, and that the jar contains 'flag-values.sam'.

Contributor

shaneknapp commented Nov 4, 2014

hmm, a quick look on that slave shows that the adam-core-0.14.1-SNAPSHOT-tests.jar is indeed there, and that the jar contains 'flag-values.sam'.

@shaneknapp

This comment has been minimized.

Show comment
Hide comment
@shaneknapp

shaneknapp Nov 4, 2014

Contributor

jenkins, test this please.

Contributor

shaneknapp commented Nov 4, 2014

jenkins, test this please.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit ac43b72 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision c5d4591611db50bc1c6d41caa619e0f3c31aedc4 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f c5d4591611db50bc1c6d41caa619e0f3c31aedc4 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit ac43b72 automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision c5d4591611db50bc1c6d41caa619e0f3c31aedc4 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f c5d4591611db50bc1c6d41caa619e0f3c31aedc4 > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 4, 2014

Member

ah, didn't see your guys' comments here and pushed another desperate attempt at a fix, this time using Thread.currentThread().getContextClassLoader instead of ClassLoader.getSystemClassLoader, based on seeing the former in Features2ADAMSuite; only later did I realized that test case is ignored and so is not necessarily a good indicator of how to get this passing!

I'm guessing the last quick jenkins failure was because I force-pushed the commit jenkins wanted to test out from under it; let's see what it does on the latest fix attempt.

Member

ryan-williams commented Nov 4, 2014

ah, didn't see your guys' comments here and pushed another desperate attempt at a fix, this time using Thread.currentThread().getContextClassLoader instead of ClassLoader.getSystemClassLoader, based on seeing the former in Features2ADAMSuite; only later did I realized that test case is ignored and so is not necessarily a good indicator of how to get this passing!

I'm guessing the last quick jenkins failure was because I force-pushed the commit jenkins wanted to test out from under it; let's see what it does on the latest fix attempt.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 4, 2014

Member

@shaneknapp one strange thing is that flag-values.sam lives in adam-core but is being called from adam-cli; could that be causing the problem? Should I try moving it to adam-cli/src/test/resources?

Member

ryan-williams commented Nov 4, 2014

@shaneknapp one strange thing is that flag-values.sam lives in adam-core but is being called from adam-cli; could that be causing the problem? Should I try moving it to adam-cli/src/test/resources?

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit 6370b6c automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision 7416b585433baf22365d127d09e689602d985f4f (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 7416b585433baf22365d127d09e689602d985f4f > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Nov 4, 2014

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

Build result: FAILURE

GitHub pull request #451 of commit 6370b6c automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/451/merge^{commit} # timeout=10Checking out Revision 7416b585433baf22365d127d09e689602d985f4f (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 7416b585433baf22365d127d09e689602d985f4f > git rev-list a7a2569f68f2981425bb14e2fffc1bdbd8024b8c # timeout=10Triggering ADAM-prb » 2.2.0,centosTriggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosADAM-prb » 2.2.0,centos completed with result FAILUREADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILURE
Test FAILed.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 4, 2014

Member

ok, a few notes:

  • I was mistaken about 357 having failed any differently from the others; I just clicked on the wrong link.
  • They're all failing with java.io.FileNotFoundException: File file:/home/jenkins/workspace/ADAM-prb/hadoop.version/2.3.0/label/centos/adam-core/target/adam-core-0.14.1-SNAPSHOT-tests.jar!/flag-values.sam does not exist
  • I don't understand how that is possible given the FileNotFoundException catch that should have been in the code for 357 and 358.
    • is it possible that jenkins is not rebuilding my code correctly? The failure should be different due to that catch block, but instead we're still seeing the same failure as we saw in 350 and 356.
  • I'm going to try putting flag-values.sam into adam-cli since that's where ViewSuite is; I'm still kind of guessing here though.
Member

ryan-williams commented Nov 4, 2014

ok, a few notes:

  • I was mistaken about 357 having failed any differently from the others; I just clicked on the wrong link.
  • They're all failing with java.io.FileNotFoundException: File file:/home/jenkins/workspace/ADAM-prb/hadoop.version/2.3.0/label/centos/adam-core/target/adam-core-0.14.1-SNAPSHOT-tests.jar!/flag-values.sam does not exist
  • I don't understand how that is possible given the FileNotFoundException catch that should have been in the code for 357 and 358.
    • is it possible that jenkins is not rebuilding my code correctly? The failure should be different due to that catch block, but instead we're still seeing the same failure as we saw in 350 and 356.
  • I'm going to try putting flag-values.sam into adam-cli since that's where ViewSuite is; I'm still kind of guessing here though.
@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

AmplabJenkins commented Nov 4, 2014

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

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 4, 2014

Member

so the issue was that the test file needed to be in adam-cli where the test itself lives. I don't fully understand why, but it seems fixed now; I cleaned up some unnecessary other cruft that I'd added while debugging this as well.

Member

ryan-williams commented Nov 4, 2014

so the issue was that the test file needed to be in adam-cli where the test itself lives. I don't fully understand why, but it seems fixed now; I cleaned up some unnecessary other cruft that I'd added while debugging this as well.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

AmplabJenkins commented Nov 4, 2014

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

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 4, 2014

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

AmplabJenkins commented Nov 4, 2014

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

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

So, SAM/BAM writing on multiple partitions doesn't work on Yarn clusters, I think. The issue I'm seeing is basically what we saw on #353, but in the BAM/SAM-writing flow instead of the VCF-writing one.

Basically, ADAMBAMOutputFormat.setHeader needs to get called on each executor before instances of ADAMBAMOutputFormat are created by PairedRDDFunctions.saveAsNewAPIHadoopDataset.

Seems like it should be a straightforward application of @fnothaft's fix from #353, but I've tried a few things at ryan/bam and nothing is working.

Specifically, I've tried to mapPartitions (with and without preservesPartitioning) and foreachPartitions a function that calls ADAMBAMOutputFormat.addHeader(hdrBcast.value) (where hdrBcast is a Broadcast of the relevant SAMFileHeader).

I've been testing on a 24-partition RDD. The failure mode is generally that I see logging consistent with the SAMFileHeader being set on 24 partitions, but then the saveAsNewAPIHadoopFile job runs on a fresh set of executors on new machines where the SAMFileHeader has not been set.

I tried calling .coalesce(1, shuffle = true) on the RDD immediately prior to this part of the code, thinking that maybe the VCF fix worked because we had generally done that before attempting to write our VCFs, and maybe having only one partition meant that the stages got grouped or run together in some way that made it work, but that didn't work for me either.

I don't really have any good leads. Does ADAM's BAM-writing flow work across multiple nodes? How does this even work in the VCF case?

Member

ryan-williams commented Nov 5, 2014

So, SAM/BAM writing on multiple partitions doesn't work on Yarn clusters, I think. The issue I'm seeing is basically what we saw on #353, but in the BAM/SAM-writing flow instead of the VCF-writing one.

Basically, ADAMBAMOutputFormat.setHeader needs to get called on each executor before instances of ADAMBAMOutputFormat are created by PairedRDDFunctions.saveAsNewAPIHadoopDataset.

Seems like it should be a straightforward application of @fnothaft's fix from #353, but I've tried a few things at ryan/bam and nothing is working.

Specifically, I've tried to mapPartitions (with and without preservesPartitioning) and foreachPartitions a function that calls ADAMBAMOutputFormat.addHeader(hdrBcast.value) (where hdrBcast is a Broadcast of the relevant SAMFileHeader).

I've been testing on a 24-partition RDD. The failure mode is generally that I see logging consistent with the SAMFileHeader being set on 24 partitions, but then the saveAsNewAPIHadoopFile job runs on a fresh set of executors on new machines where the SAMFileHeader has not been set.

I tried calling .coalesce(1, shuffle = true) on the RDD immediately prior to this part of the code, thinking that maybe the VCF fix worked because we had generally done that before attempting to write our VCFs, and maybe having only one partition meant that the stages got grouped or run together in some way that made it work, but that didn't work for me either.

I don't really have any good leads. Does ADAM's BAM-writing flow work across multiple nodes? How does this even work in the VCF case?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

@ryan-williams you're always bringing the fun problems. ;)

Something is screwy with the way metadata is being handled in Hadoop-BAM. We shouldn't be facing these problems every way we try to export data. I can probably take some time to look at this on Friday or thereabouts. It's so trivially simple to do SAM/BAM/VCF export that it's not clear to me whether it is worth debugging why this problem continues to happen in Hadoop-BAM (because debugging the VCF stuff was a nightmare) vs. just replacing Hadoop-BAM in ADAM.

Member

fnothaft commented Nov 5, 2014

@ryan-williams you're always bringing the fun problems. ;)

Something is screwy with the way metadata is being handled in Hadoop-BAM. We shouldn't be facing these problems every way we try to export data. I can probably take some time to look at this on Friday or thereabouts. It's so trivially simple to do SAM/BAM/VCF export that it's not clear to me whether it is worth debugging why this problem continues to happen in Hadoop-BAM (because debugging the VCF stuff was a nightmare) vs. just replacing Hadoop-BAM in ADAM.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

As an aside though, I haven't run into any trouble doing multi-node SAM/BAM export over here. We're not running Yarn though. Not sure if Yarn's the troublemaker or not, but if a file format fails randomly because of the scheduler used, then it is a bad file format.

Member

fnothaft commented Nov 5, 2014

As an aside though, I haven't run into any trouble doing multi-node SAM/BAM export over here. We're not running Yarn though. Not sure if Yarn's the troublemaker or not, but if a file format fails randomly because of the scheduler used, then it is a bad file format.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

I try!

Interesting; what metadata should Hadoop-BAM be handling and how should it be keeping us safe from these errors?

To me the screwy thing seems to be Spark's insistence on instantiating OutputFormat via a nullary constructor. It seems like we should be able to add a hook in there to broadcast out something to initialize our OutputFormat instance with, if Spark would let us. Thoughts?

Member

ryan-williams commented Nov 5, 2014

I try!

Interesting; what metadata should Hadoop-BAM be handling and how should it be keeping us safe from these errors?

To me the screwy thing seems to be Spark's insistence on instantiating OutputFormat via a nullary constructor. It seems like we should be able to add a hook in there to broadcast out something to initialize our OutputFormat instance with, if Spark would let us. Thoughts?

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

Yea, I am in the state of debugging where I don't understand how this could ever work. I suppose if your scheduler allows you to initialize some singletons on each node and then ensures that all subsequent executors run in places where that initialization has taken place, you would be ok...?

Member

ryan-williams commented Nov 5, 2014

Yea, I am in the state of debugging where I don't understand how this could ever work. I suppose if your scheduler allows you to initialize some singletons on each node and then ensures that all subsequent executors run in places where that initialization has taken place, you would be ok...?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

To me the screwy thing seems to be Spark's insistence on instantiating OutputFormat via a nullary constructor. It seems like we should be able to add a hook in there to broadcast out something to initialize our OutputFormat instance with, if Spark would let us. Thoughts?

I won't lie; I'm not much of a Hadoop person, but isn't this directly inherited from Hadoop? But, agreed; the nullary constructor is an... odd choice.

Interesting; what metadata should Hadoop-BAM be handling and how should it be keeping us safe from these errors?

Haven't looked too much under the hood, but most other output formats that have some sort of global state (like Parquet) seem to do this correctly.

Member

fnothaft commented Nov 5, 2014

To me the screwy thing seems to be Spark's insistence on instantiating OutputFormat via a nullary constructor. It seems like we should be able to add a hook in there to broadcast out something to initialize our OutputFormat instance with, if Spark would let us. Thoughts?

I won't lie; I'm not much of a Hadoop person, but isn't this directly inherited from Hadoop? But, agreed; the nullary constructor is an... odd choice.

Interesting; what metadata should Hadoop-BAM be handling and how should it be keeping us safe from these errors?

Haven't looked too much under the hood, but most other output formats that have some sort of global state (like Parquet) seem to do this correctly.

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

Yea, I am in the state of debugging where I don't understand how this could ever work. I suppose if your scheduler allows you to initialize some singletons on each node and then ensures that all subsequent executors run in places where that initialization has taken place, you would be ok...?

@ryan-williams come back... just walk... walk towards the light! Walk away from the Darkness Ryan!

Member

fnothaft commented Nov 5, 2014

Yea, I am in the state of debugging where I don't understand how this could ever work. I suppose if your scheduler allows you to initialize some singletons on each node and then ensures that all subsequent executors run in places where that initialization has taken place, you would be ok...?

@ryan-williams come back... just walk... walk towards the light! Walk away from the Darkness Ryan!

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

OK, I am back. Can you say more about how you think this works on your multi-node setups? How does this singleton state get set in each worker's JVM? Does it? Are you inadvertently side-stepping the issue somehow?

Member

ryan-williams commented Nov 5, 2014

OK, I am back. Can you say more about how you think this works on your multi-node setups? How does this singleton state get set in each worker's JVM? Does it? Are you inadvertently side-stepping the issue somehow?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

@ryan-williams well, since we're not running Yarn, we never have the case where the foreachPartition call and the saveAsNewAPIHadoopFile call occur on different sets of executors. As you may imagine, that greatly simplifies the problem. ;)

Member

fnothaft commented Nov 5, 2014

@ryan-williams well, since we're not running Yarn, we never have the case where the foreachPartition call and the saveAsNewAPIHadoopFile call occur on different sets of executors. As you may imagine, that greatly simplifies the problem. ;)

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

that makes sense, however, the existing code only calls ADAMBAMOutputFormat.addHeader once, on one node, AFAICT; is this not a problem in your multi-node setup? Does every node execute the "driver" code paths in your setup?

Member

ryan-williams commented Nov 5, 2014

that makes sense, however, the existing code only calls ADAMBAMOutputFormat.addHeader once, on one node, AFAICT; is this not a problem in your multi-node setup? Does every node execute the "driver" code paths in your setup?

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

@ryan-williams ah, good point.

Does every node execute the "driver" code paths in your setup?

It shouldn't...

Honestly, I'd have to double check this. Not 100% sure what's going on off of the top of my head...

Member

fnothaft commented Nov 5, 2014

@ryan-williams ah, good point.

Does every node execute the "driver" code paths in your setup?

It shouldn't...

Honestly, I'd have to double check this. Not 100% sure what's going on off of the top of my head...

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 5, 2014

Member

haha! come to the dark side frank... maybe this has never worked :)

Member

ryan-williams commented Nov 5, 2014

haha! come to the dark side frank... maybe this has never worked :)

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Nov 5, 2014

Member

@ryan-williams haha yeah, no kidding. You've got me doubting my own sanity...

But, I may have been doing so already. ;)

Member

fnothaft commented Nov 5, 2014

@ryan-williams haha yeah, no kidding. You've got me doubting my own sanity...

But, I may have been doing so already. ;)

ryan-williams added some commits Nov 3, 2014

add adam `view` command
similar to `samtools view`
add "disjunction" -g / -G options to `adam view`
-f and -F do positive and negative conjunctions, but in at least one
situation I've wanted to do an "or": give me all reads in this file
where the read is unmapped *or* the mate is unmapped.
Add a test for new `view` command
- includes a SAM file with one read for each of the 700 possible
  flag-field values allowed by HTSJDK’s and ADAM’s SAM parsing code
  paths.
- bonus: includes `make-flag-values-sam.py` script used to generate the
  aforementioned SAM file.
- concern: ViewSuite test takes about 30s to run on my laptop; how much
  do we care about test-running latency? Is there an obviously quicker
  way to run a similar test to what I have here?
@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 6, 2014

Member

Alright, I've retooled this a bit and I think it's ready to go independent of the "can we really output multi-shard SAMs/BAMs" rabbit-hole.

  • I've been able to run it on my cluster outputting ADAM format and verify that it works and performs as desired.
  • I added -g and -G flags (analogous to -f and -F) that allow for selecting disjunctions of flag values.
    • example I've been using it for: filter to reads that are not mapped or whose mate is not mapped, using -g 12 (0x4 | 0x8 == 12)
  • I have some pretty extensive test cases and documentation of them, based around flag-values.sam which is one read for each of the 700 values in [0, 2^12) that HTSJDK and ADAM accept as an internally-consistent set of the 12 flag bits.

Let me know if you have other thoughts / concerns but I think we can get this in while we think about sharded-SAM-writing on the backburner.

Member

ryan-williams commented Nov 6, 2014

Alright, I've retooled this a bit and I think it's ready to go independent of the "can we really output multi-shard SAMs/BAMs" rabbit-hole.

  • I've been able to run it on my cluster outputting ADAM format and verify that it works and performs as desired.
  • I added -g and -G flags (analogous to -f and -F) that allow for selecting disjunctions of flag values.
    • example I've been using it for: filter to reads that are not mapped or whose mate is not mapped, using -g 12 (0x4 | 0x8 == 12)
  • I have some pretty extensive test cases and documentation of them, based around flag-values.sam which is one read for each of the 700 values in [0, 2^12) that HTSJDK and ADAM accept as an internally-consistent set of the 12 flag bits.

Let me know if you have other thoughts / concerns but I think we can get this in while we think about sharded-SAM-writing on the backburner.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 6, 2014

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

AmplabJenkins commented Nov 6, 2014

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

@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Nov 7, 2014

Member

This LGTM. @ryan-williams and @fnothaft is this ready to merge?

Member

massie commented Nov 7, 2014

This LGTM. @ryan-williams and @fnothaft is this ready to merge?

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 7, 2014

Member

I think it is on my end, not working on it actively atm and will do so in a new PR if I start to.

Member

ryan-williams commented Nov 7, 2014

I think it is on my end, not working on it actively atm and will do so in a new PR if I start to.

massie added a commit that referenced this pull request Nov 7, 2014

Merge pull request #451 from ryan-williams/view
add `adam view` command, analogous to `samtools view`

@massie massie merged commit 51c4b24 into bigdatagenomics:master Nov 7, 2014

1 check passed

default Merged build finished.
Details
@massie

This comment has been minimized.

Show comment
Hide comment
@massie

massie Nov 7, 2014

Member

Thanks, Ryan!

Member

massie commented Nov 7, 2014

Thanks, Ryan!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment