[ADAM-1883] Python and R caching #1885

akmorrow13 · 2018-01-24T05:56:20Z

No description provided.

coveralls · 2018-01-24T06:10:16Z

Coverage decreased (-0.05%) to 82.728% when pulling b11d7c2 on akmorrow13:python-caching into c5b4ebc on bigdatagenomics:master.

coveralls · 2018-01-24T06:10:17Z

Coverage decreased (-0.05%) to 82.728% when pulling b11d7c2 on akmorrow13:python-caching into c5b4ebc on bigdatagenomics:master.

coveralls · 2018-01-24T06:10:17Z

Coverage decreased (-0.2%) to 82.561% when pulling 129f617 on akmorrow13:python-caching into eb3528c on bigdatagenomics:master.

AmplabJenkins · 2018-01-24T06:25:22Z

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

akmorrow13 · 2018-01-24T17:28:51Z

Resolves #1883

akmorrow13 · 2018-01-24T18:16:54Z

This is tested and works like so:

reads = ac.loadAlignments(readsPath, stringency=LENIENT)
reads.__jvmRdd.cache()
reads.toDF().count() # caching
reads.toDF().count() # is cached, will be fast
reads._jvmRdd.unpersist()

heuermh · 2018-01-24T18:21:41Z

I'm not sure I'm comfortable reviewing Python stuff yet. Is it reasonable to expose reads.__jvmRdd. to the caller like this? And isn't toDF().rdd contradictory?

AmplabJenkins · 2018-01-24T18:23:53Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 1c40800 # timeout=10Checking out Revision 1c40800 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 1c40800b3b7b8e34bbc320651ff03f1dcc7c1e12First time build. Skipping changelog.Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

akmorrow13 · 2018-01-24T18:27:59Z

@heuermh

_jvmRDD has already been exposed, this PR does not expose that.
I have added no code calling .toDF().rdd, so I am not sure what problem there is there. This is the most straightforward way to access the pyspark RDD currently. We could add a better method for accessing this but that is a different problem then the one we are addressing here.

heuermh · 2018-01-24T18:49:20Z

re 2 I'm asking is the toDF() necessary in your comment above? Seems odd to convert to data frame and then back to rdd.

akmorrow13 · 2018-01-24T18:50:57Z

I agree @heuermh, but this is currently the only way to access a pyspark rdd from our API. The other rdd option is ._jvmRdd, but this is not a pyspark RDD, just an inaccessible Java Member.

AmplabJenkins · 2018-01-24T18:51:14Z

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

heuermh · 2018-01-24T18:52:25Z

I see, thanks

akmorrow13 · 2018-01-24T18:58:51Z

@heuermh I updated the count example to exclude .rdd so it isn't so confusing

fnothaft

Thanks for doing this @akmorrow13! Two things:

Can you add a persist(sl: StorageLevel) method to GenomicRDD?
Can you add the persist/cache/unpersist methods on the Python (and R) GenomicRDD classes? cache and unpersist should be trivial but persist will take a tiny amount more finesse.

To wit:

_jvmRDD has already been exposed, this PR does not expose that.

_ is Python convention for "protected", so don't expect users to call that. Instead, to GenomicRDD add:

  def cache(self):

     self._jvmRdd.cache()

fnothaft · 2018-01-25T03:47:43Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

@@ -137,6 +137,26 @@ trait GenomicRDD[T, U <: GenomicRDD[T, U]] extends Logging {
   */
  def replaceSequences(newSequences: SequenceDictionary): U

+  /**
+   * Caches underlying RDD in memory. This interfaced is used to


Drop comment about R/Python, this method is useful independent of that.

fnothaft · 2018-01-25T03:47:53Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+
+  /**
+   * Unpersists underlying RDD from memory. This interfaced is used to
+   * access caching functionality from the python and R APIs.


Drop comment about R/Python, this method is useful independent of that.

heuermh · 2018-01-25T17:12:57Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+   * @return type of RDD that was cached
+   */
+  def cache() = {
+    rdd.cache()


Asking out of ignorance, does caching make any sense for GenomicDataset.dataset?

Ah, yes... These should probably be overridden by the DatasetBound...RDD classes.

@fnothaft @heuermh I am confused what this means. Do you want me to implement this function in every DatasetBound...RDD function? There seems to be a lot of them, while this approach just requires 1 implementation.

Hi @akmorrow13! Sorry for the slow reply here. Yes, that is what I want, but the simpler way to do this is to have RDDBoundGenomicRDD and DatasetBoundGenomicRDD traits that both implement the cache, persist, and unpersist methods. Then RDDBoundAlignmentRecordRDD would extend RDDBoundGenomicRDD, etc. We should do this for saveAsParquet as well.

Also, my preference is that unpersist returns Unit (no return value) and that cache/persist return either U or Unit.

akmorrow13 · 2018-01-25T19:03:09Z

I added a persist function, but I have absolutely no idea to handle passing parameters from python to scala. I tried following the example for stringency, like this:

self._jvmRdd.persist(self.sc._jvm.org.apache.spark.storage.StorageLevel.apply(sl_dict["useDisk"], sl_dict["useMemory"],
                             sl_dict["deserialized"], sl_dict["replication"]))

but that didn't seem to work

AmplabJenkins · 2018-01-25T19:30:52Z

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

AmplabJenkins · 2018-01-25T20:15:53Z

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

fnothaft · 2018-01-25T20:31:31Z

adam-python/bdgenomics/adam/rdd.py

+    def cache(self):
+        self._jvmRdd.cache()
+
+    def persist(self, sl):


sl should be of type StorageLevel

Use the Java StorageLevels.create method

Voila:

self._jvmRdd.persist(self.sc._jvm.org.apache.spark.api.StorageLevels.create(sl.useDisk, sl.useMemory, sl.useOffHeap, sl.useDeserialized, sl.replication)

Py4j lines Python types up with Java types, so you need to line Python bool up with Java boolean, not Scala.Boolean, Python int up with Java int, not Scala.Integer, etc.

AmplabJenkins · 2018-01-25T20:39:26Z

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

AmplabJenkins · 2018-01-25T23:07:00Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 52c468a # timeout=10Checking out Revision 52c468a (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 52c468a > /home/jenkins/git2/bin/git rev-list 92d412d # timeout=10Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result SUCCESSADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result SUCCESSADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

AmplabJenkins · 2018-01-25T23:41:20Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 2a5c7d4 # timeout=10Checking out Revision 2a5c7d4 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 2a5c7d4 > /home/jenkins/git2/bin/git rev-list 52c468a # timeout=10Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

akmorrow13 · 2018-01-26T01:16:36Z

Jenkins, retest this please.

AmplabJenkins · 2018-01-26T01:36:43Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 2a5c7d4 # timeout=10Checking out Revision 2a5c7d4 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 2a5c7d4 > /home/jenkins/git2/bin/git rev-list 2a5c7d4 # timeout=10Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

AmplabJenkins · 2018-01-26T01:55:31Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 01bd9f7 # timeout=10Checking out Revision 01bd9f7 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 01bd9f7 > /home/jenkins/git2/bin/git rev-list 2a5c7d4 # timeout=10Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

AmplabJenkins · 2018-01-26T05:11:19Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains b38a1c5 # timeout=10Checking out Revision b38a1c5 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f b38a1c5 > /home/jenkins/git2/bin/git rev-list 01bd9f7 # timeout=10Triggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

akmorrow13 · 2018-01-26T05:32:47Z

Jenkins, retest this please.

fnothaft · 2018-02-06T21:37:01Z

adam-python/bdgenomics/adam/rdd.py

+        Caches underlying RDD in memory.
+        """
+
+        self._jvmRdd.cache()


Similarly, these should replace the RDD.

fnothaft · 2018-02-06T21:37:06Z

adam-python/bdgenomics/adam/rdd.py

+        :param sl new StorageLevel
+        """
+
+        self._jvmRdd.persist(self.sc._jvm.org.apache.spark.api.java.StorageLevels.create(sl.useDisk, \


Similarly, these should replace the RDD.

fnothaft · 2018-02-06T21:37:11Z

adam-python/bdgenomics/adam/rdd.py

+        Unpersists underlying RDD from memory.
+        """
+
+        self._jvmRdd.unpersist()


Similarly, these should replace the RDD.

fnothaft · 2018-02-06T21:37:22Z

adam-r/bdgenomics.adam/R/rdd.R

+setMethod("cache",
+          signature(ardd = "GenomicRDD"),
+          function(ardd) {
+              sparkR.callJMethod(ardd@jrdd, "cache")


Similarly, these should replace the RDD.

fnothaft · 2018-02-06T21:37:27Z

adam-r/bdgenomics.adam/R/rdd.R

+                    sl = "character"),
+          function(ardd, sl) {
+              storageLevel <- sparkR.callJStatic("org.apache.spark.storage.StorageLevel", "fromString", sl)
+              sparkR.callJMethod(ardd@jrdd, "persist", storageLevel)


Similarly, these should replace the RDD.

fnothaft · 2018-02-06T21:37:32Z

adam-r/bdgenomics.adam/R/rdd.R

+setMethod("unpersist",
+          signature(ardd = "GenomicRDD"),
+          function(ardd) {
+              sparkR.callJMethod(ardd@jrdd, "unpersist")


Similarly, these should replace the RDD.

AmplabJenkins · 2018-02-07T01:33:25Z

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

fnothaft

Just a couple of stylistic nits. Looks good otherwise; thanks @akmorrow13!

fnothaft · 2018-02-07T17:03:32Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/feature/FeatureRDD.scala

-  JavaSaveArgs,
-  SAMHeaderWriter
-}
+import org.bdgenomics.adam.rdd.{ DatasetBoundGenomicDataset, AvroGenomicRDD, JavaSaveArgs, SAMHeaderWriter }


Nit: Please split import across multiple lines.

fnothaft · 2018-02-07T17:04:08Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/fragment/FragmentRDD.scala

@@ -30,7 +30,7 @@ import org.bdgenomics.adam.models.{
  SequenceDictionary
 }
 import org.bdgenomics.adam.rdd.ADAMContext._
-import org.bdgenomics.adam.rdd.{ AvroRecordGroupGenomicRDD, JavaSaveArgs }
+import org.bdgenomics.adam.rdd.{ DatasetBoundGenomicDataset, AvroRecordGroupGenomicRDD, JavaSaveArgs }


Please split long import line across multiple lines.

fnothaft · 2018-02-07T17:04:40Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/variant/VariantRDD.scala

-  AvroGenomicRDD,
-  VCFHeaderUtils
-}
+import org.bdgenomics.adam.rdd.{ DatasetBoundGenomicDataset, AvroGenomicRDD, VCFHeaderUtils }


Nit: please split long import across multiple lines.

fnothaft · 2018-02-07T17:07:57Z

adam-python/bdgenomics/adam/rdd.py

+        :param sl new StorageLevel
+        """
+
+        return self._replaceRdd(self._jvmRdd.persist(self.sc._jvm.org.apache.spark.api.java.StorageLevels.create(sl.useDisk, \


This line is really long. Can you split it out as:

jsl = self.sc._jvm.org.apache.spark.api.java.StorageLevels.create(sl.useDisk, sl.useMemory, sl.useOffHeap, sl.deserialized, sl.replication) return self._replaceRdd(self._jvmRdd.persist(jsl))

Indented parameters should align with the opening delimiter.

fnothaft · 2018-02-07T17:17:19Z

adam-python/bdgenomics/adam/test/alignmentRecordRdd_test.py

@@ -201,3 +202,33 @@ def test_filterByOverlappingRegions(self):

        filtered = reads.filterByOverlappingRegions(querys)
        self.assertEquals(filtered.toDF().count(), 2)
+
+    def test_caching(self):


Nit: 2 lines of space before each new function definition.

AmplabJenkins · 2018-02-07T18:20:32Z

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

AmplabJenkins · 2018-02-07T18:55:38Z

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

AmplabJenkins · 2018-02-07T19:30:36Z

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

heuermh

Looks good, some minor doc fixes, and a question about GenomicDatset.

heuermh · 2018-02-07T19:52:03Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+  }
+
+  /**
+   * Persists underlying RDD in memory.


Persists underlying RDD in memory. → Persists underlying RDD in memory or disk.

heuermh · 2018-02-07T19:52:28Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+  }
+
+  /**
+   * Unpersists underlying RDD from memory.


Unpersists underlying RDD from memory. → Unpersists underlying RDD from memory or disk.

heuermh · 2018-02-07T19:54:22Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+/**
+ * A trait describing a GenomicDataset that also supports the Spark SQL APIs.
+ */
+trait DatasetBoundGenomicDataset[T, U <: Product, V <: GenomicDataset[T, U, V]] extends GenomicDataset[T, U, V] {


@fnothaft I think you may have suggested this, doesn't GenomicDataset already imply the RDD has been bound to a dataset? Couldn't these be moved there?

I'm not very sure what GenomicDataset is for, it isn't used everywhere I would've thought
https://github.com/bigdatagenomics/adam/search?utf8=%E2%9C%93&q=GenomicDataset&type=

No, GenomicDataset is any GenomicRDD where the underlying genomic data can be viewed as both an RDD or a Dataset, while the base GenomicRDD trait only provides RDDs. This distinction will go away soon.

ParquetUnbound/RDDBound/DatasetBound implies how the data is currently physically represented. Parquet unbound means that the data hasn't been materialized into memory yet and that the data is represented using a Parquet file, while RDD/DatasetBound means that the current (as of the last transformation) representation is an RDD/Dataset.

Since cache impacts the physical materialization of the data, we want the new DatasetBoundGenomicDataset abstract class to override cache, and not the GenomicDataset abstract class.

heuermh · 2018-02-07T19:54:56Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+  }
+
+  /**
+   * Persists underlying dataset in memory.


Persists underlying dataset in memory. → Persists underlying dataset in memory or disk.

heuermh · 2018-02-07T19:55:23Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+   * Persists underlying dataset in memory.
+   *
+   * @param sl new StorageLevel
+   * @return Unit


@return Unit → @return V cached GenomicDataset

heuermh · 2018-02-07T19:55:40Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+  }
+
+  /**
+   * Unpersists underlying dataset from memory.


Unpersists underlying dataset from memory. → Unpersists underlying dataset from memory or disk.

heuermh · 2018-02-07T19:56:05Z

adam-core/src/main/scala/org/bdgenomics/adam/rdd/GenomicRDD.scala

+  /**
+   * Unpersists underlying dataset from memory.
+   *
+   * @return Unit


@return Unit → @return V uncached GenomicDataset

unpersisted, not uncached

heuermh · 2018-02-07T19:57:55Z

adam-python/bdgenomics/adam/rdd.py

+
+    def persist(self, sl):
+        """
+        Persists underlying RDD in memory.


Persists underlying RDD in memory. → Persists underlying RDD in memory or disk.

heuermh · 2018-02-07T19:58:12Z

adam-python/bdgenomics/adam/rdd.py

+
+    def unpersist(self):
+        """
+        Unpersists underlying RDD from memory.


Unpersists underlying RDD from memory. → Unpersists underlying RDD from memory or disk.

AmplabJenkins · 2018-02-08T00:28:28Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains c71ccf4 # timeout=10Checking out Revision c71ccf4 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f c71ccf4 > /home/jenkins/git2/bin/git rev-list e719c88 # timeout=10Triggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

akmorrow13 · 2018-02-08T15:44:21Z

Jenkins, retest this please.

akmorrow13 · 2018-02-08T15:45:40Z

Jenkins, retest this please.

AmplabJenkins · 2018-02-08T16:05:29Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains c71ccf4 # timeout=10Checking out Revision c71ccf4 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f c71ccf4 > /home/jenkins/git2/bin/git rev-list c71ccf4 # timeout=10Triggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

fnothaft · 2018-02-09T17:47:28Z

adam-python/bdgenomics/adam/rdd.py

+        :return: Returns a new, persisted RDD.
+        """
+
+        jsl = self.sc._jvm.org.apache.spark.api.java.StorageLevels.create(sl.useDisk, \


Do not use \ to break lines, and please indent to the opening delimiter. I.e., every sl should be at the same character position on the line.

fnothaft · 2018-02-09T17:47:35Z

adam-python/bdgenomics/adam/rdd.py

+
+        return self._replaceRdd(self._jvmRdd.persist(jsl))
+
+    def unpersist(self):


Two spaces above.

fnothaft · 2018-02-09T17:50:01Z

From looking at Jenkins, needs a ./scripts/format-sources.

AmplabJenkins · 2018-02-09T18:58:21Z

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /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/1885/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 89131e6 # timeout=10Checking out Revision 89131e6 (origin/pr/1885/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 89131e6 > /home/jenkins/git2/bin/git rev-list c71ccf4 # timeout=10Triggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

akmorrow13 · 2018-02-09T19:06:17Z

Jenkins, retest this please.

AmplabJenkins · 2018-02-09T20:42:01Z

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

akmorrow13 · 2018-02-14T00:59:08Z

I think this is all set @fnothaft let me know if there is anything else that is needed

AmplabJenkins · 2018-02-14T20:14:49Z

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

fnothaft · 2018-02-14T20:26:40Z

Thanks @akmorrow13! Merged manually as 6051321.

akmorrow13 force-pushed the python-caching branch from 931c44f to 2e77fb1 Compare January 24, 2018 18:24

fnothaft requested changes Jan 25, 2018

View reviewed changes

heuermh reviewed Jan 25, 2018

View reviewed changes

fnothaft requested changes Jan 25, 2018

View reviewed changes

fnothaft requested changes Feb 6, 2018

View reviewed changes

akmorrow13 added this to the 0.24.0 milestone Feb 7, 2018

akmorrow13 force-pushed the python-caching branch from bff842b to 164c067 Compare February 7, 2018 17:40

fnothaft requested changes Feb 7, 2018

View reviewed changes

akmorrow13 force-pushed the python-caching branch 2 times, most recently from 863b906 to 0fdbdf7 Compare February 7, 2018 18:07

heuermh requested changes Feb 7, 2018

View reviewed changes

akmorrow13 force-pushed the python-caching branch from 0fdbdf7 to 026ffad Compare February 8, 2018 00:11

fnothaft requested changes Feb 9, 2018

View reviewed changes

heuermh approved these changes Feb 12, 2018

View reviewed changes

added caching functions for python

129f617

akmorrow13 force-pushed the python-caching branch from 36d11d5 to 129f617 Compare February 14, 2018 19:35

fnothaft approved these changes Feb 14, 2018

View reviewed changes

fnothaft closed this Feb 14, 2018

heuermh changed the title ~~Python and R caching~~ [ADAM-1883] Python and R caching Feb 14, 2018


		return self._replaceRdd(self._jvmRdd.persist(jsl))

		def unpersist(self):

[ADAM-1883] Python and R caching #1885

[ADAM-1883] Python and R caching #1885

Conversation

akmorrow13 commented Jan 24, 2018

coveralls commented Jan 24, 2018

coveralls commented Jan 24, 2018

coveralls commented Jan 24, 2018 • edited Loading

AmplabJenkins commented Jan 24, 2018

akmorrow13 commented Jan 24, 2018

akmorrow13 commented Jan 24, 2018 • edited Loading

heuermh commented Jan 24, 2018 • edited Loading

AmplabJenkins commented Jan 24, 2018

Build result: FAILURE

akmorrow13 commented Jan 24, 2018

heuermh commented Jan 24, 2018

akmorrow13 commented Jan 24, 2018

AmplabJenkins commented Jan 24, 2018

heuermh commented Jan 24, 2018

akmorrow13 commented Jan 24, 2018

fnothaft left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akmorrow13 commented Jan 25, 2018 • edited Loading

AmplabJenkins commented Jan 25, 2018

AmplabJenkins commented Jan 25, 2018

Choose a reason for hiding this comment

AmplabJenkins commented Jan 25, 2018

AmplabJenkins commented Jan 25, 2018

Build result: FAILURE

AmplabJenkins commented Jan 25, 2018

Build result: FAILURE

akmorrow13 commented Jan 26, 2018

AmplabJenkins commented Jan 26, 2018

Build result: FAILURE

AmplabJenkins commented Jan 26, 2018

Build result: FAILURE

AmplabJenkins commented Jan 26, 2018

Build result: FAILURE

akmorrow13 commented Jan 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Feb 7, 2018

fnothaft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Feb 7, 2018

AmplabJenkins commented Feb 7, 2018

AmplabJenkins commented Feb 7, 2018

heuermh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Feb 8, 2018

Build result: FAILURE

akmorrow13 commented Feb 8, 2018

akmorrow13 commented Feb 8, 2018

AmplabJenkins commented Feb 8, 2018

coveralls commented Jan 24, 2018 •

edited

Loading

akmorrow13 commented Jan 24, 2018 •

edited

Loading

heuermh commented Jan 24, 2018 •

edited

Loading

fnothaft left a comment •

edited

Loading

akmorrow13 commented Jan 25, 2018 •

edited

Loading