use ParsedLoci in loadIndexedBam #1277

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@ryan-williams
Member

ryan-williams commented Nov 17, 2016

I recently factored a small library for parsing string-representations of (possibly open-ended, e.g. chr1, chr1:100-) loci ranges out of guacamole and over to https://github.com/hammerlab/genomic-loci, because I was preparing to use it across a few of my projects, and in particular wanted it plus adam-core's loadIndexedBam functionality, which this PR adds.

@fnothaft you mentioned there is some similar loci-parsing code laying around somewhere in BDG, lmk if I should find/consider it instead / in addition to this.

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Nov 17, 2016

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

Build result: FAILURE

[...truncated 38 lines...]Triggering ADAM-prb ? 2.6.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.6.1,centosADAM-prb ? 2.3.0,2.11,1.5.2,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.5.2,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,2.0.0,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.6.1,centos completed with result SUCCESSNotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

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

Build result: FAILURE

[...truncated 38 lines...]Triggering ADAM-prb ? 2.6.0,2.11,1.3.1,centosTriggering ADAM-prb ? 2.3.0,2.11,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,2.0.0,centosTriggering ADAM-prb ? 2.3.0,2.10,1.4.1,centosTriggering ADAM-prb ? 2.6.0,2.11,1.6.1,centosADAM-prb ? 2.3.0,2.11,1.5.2,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.5.2,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.10,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,2.0.0,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.10,1.6.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.3.1,centos completed with result SUCCESSADAM-prb ? 2.3.0,2.11,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,2.0.0,centos completed with result FAILUREADAM-prb ? 2.3.0,2.10,1.4.1,centos completed with result SUCCESSADAM-prb ? 2.6.0,2.11,1.6.1,centos completed with result SUCCESSNotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@ryan-williams ryan-williams referenced this pull request Nov 17, 2016

Closed

some style nits #1278

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 17, 2016

Member

Jenkins, retest this please.

SPARK_VERSION=2.0.0 SCALAVER=2.11 HADOOP_VERSION=2.6.0 ./scripts/jenkins-test

passes for me locally. In particular, the error in the logs:

[ERROR] Failed to execute goal on project adam-core-spark2_2.11: Could not resolve dependencies for project org.bdgenomics.adam:adam-core-spark2_2.11:jar:0.20.1-SNAPSHOT: The following artifacts could not be resolved: org.spire-math:spire-macros_2.11:jar:0.11.0, org.typelevel:machinist_2.11:jar:0.4.1: Could not find artifact org.spire-math:spire-macros_2.11:jar:0.11.0 -> [Help 1]

seems erroneous, as org.spire-math:spire-macros_2.11:jar:0.11.0 exists on Maven Central.

Member

ryan-williams commented Nov 17, 2016

Jenkins, retest this please.

SPARK_VERSION=2.0.0 SCALAVER=2.11 HADOOP_VERSION=2.6.0 ./scripts/jenkins-test

passes for me locally. In particular, the error in the logs:

[ERROR] Failed to execute goal on project adam-core-spark2_2.11: Could not resolve dependencies for project org.bdgenomics.adam:adam-core-spark2_2.11:jar:0.20.1-SNAPSHOT: The following artifacts could not be resolved: org.spire-math:spire-macros_2.11:jar:0.11.0, org.typelevel:machinist_2.11:jar:0.4.1: Could not find artifact org.spire-math:spire-macros_2.11:jar:0.11.0 -> [Help 1]

seems erroneous, as org.spire-math:spire-macros_2.11:jar:0.11.0 exists on Maven Central.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Nov 17, 2016

Member

Open ended range support in was added in #1252.

Member

heuermh commented Nov 17, 2016

Open ended range support in was added in #1252.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 17, 2016

Member

Cool, gtk; the ParsedLoci class I'm using here comes with some facilities for parsing to/from strings/files that I think are still useful for what I am after here, fwiw.

Member

ryan-williams commented Nov 17, 2016

Cool, gtk; the ParsedLoci class I'm using here comes with some facilities for parsing to/from strings/files that I think are still useful for what I am after here, fwiw.

@heuermh

This comment has been minimized.

Show comment
Hide comment
@heuermh

heuermh Nov 21, 2016

Member

While I am supportive of the functionality proposed here, I would have to -1 it as is with the new dependency. For better or worse, we need to cross-build for Spark 1.x/2.x and Scala 2.10/2.11 and so need upstream dependencies to do the same.

Would there be much of a performance hit to transform
ParsedLociIterable[ReferenceRegion]Iterable[LocatableReferenceRegion]

as compared to
ParsedLociLociSetlociSet.toHtsJDKIntervals?

Member

heuermh commented Nov 21, 2016

While I am supportive of the functionality proposed here, I would have to -1 it as is with the new dependency. For better or worse, we need to cross-build for Spark 1.x/2.x and Scala 2.10/2.11 and so need upstream dependencies to do the same.

Would there be much of a performance hit to transform
ParsedLociIterable[ReferenceRegion]Iterable[LocatableReferenceRegion]

as compared to
ParsedLociLociSetlociSet.toHtsJDKIntervals?

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Nov 21, 2016

Member

The genomic-loci library is published for Scala 2.1[01] and I'm not sure uses anything that would differ between Spark 1 and 2 but I'll dual-publish on that axis as well if I find that that's the case.

As it currently stands it needs some guava-shading that I haven't worked out yet to run happily inside a Spark app so this PR is not ready to merge, closing for now.

Member

ryan-williams commented Nov 21, 2016

The genomic-loci library is published for Scala 2.1[01] and I'm not sure uses anything that would differ between Spark 1 and 2 but I'll dual-publish on that axis as well if I find that that's the case.

As it currently stands it needs some guava-shading that I haven't worked out yet to run happily inside a Spark app so this PR is not ready to merge, closing for now.

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