Add Spark 2.x support with binary compatibility to 1.X via maven prof… #79
Conversation
|
Hi, please review/merge this PR. Thanks! |
|
Thanks for the reminder. Haven't got to it yet. Will do soon. On Thu, Jun 16, 2016 at 10:48 AM, StephenBoesch notifications@github.com
|
|
I get the following error after merging you request locally: 2016-06-19 20:43:17,543 [dag-scheduler-event-loop] INFO scheduler.DAGScheduler - ResultStage 0 (saveAsSequenceFile at SourceTest.scala:45) finished in 0.117 s |
|
OK I am checking out to yet another local directory (had already done so) to re-test. |
|
Thanks. Just checkout master and run completely. After that merge your pull On Sun, Jun 19, 2016 at 8:20 PM, StephenBoesch notifications@github.com
|
|
I don't follow: these are unmerged changes correct? So they would not be in trunk/master ..? I was just planning to re-check out my own branch. |
|
I merged your pull request locally into master and did a make build: git checkout -b javadba-sparkv2_v1compatible mastergit pull Step 2: Merge the changes and update on GitHub. git checkout mastergit merge --no-ff javadba-sparkv2_v1compatible On Sun, Jun 19, 2016 at 8:23 PM, StephenBoesch notifications@github.com
|
|
OK - but that is still only visible to you - correct? I mean in any case it is not on github. In the meantime - from my own javadba/CaffeOnSpark in the sparkv2_v1compatible branch - caffe is taking its time to build inside caffe-public. Will get back after that completes and I can see what caffe-grid is doing. |
|
|
|
yes it is not on github. I am trying out locally first On Sun, Jun 19, 2016 at 8:34 PM, StephenBoesch notifications@github.com
|
|
So I need your help to understand what it is the process for verification. The process I had followed was to check out my own branch again to a distinct/new directory. Then do git clone https://github.com/javadba/CaffeOnSpark caos mvn package test inside caffe-public. |
|
OK good . Your prior post about "checkout master" is misleading - so now I think we're on same page. |
|
I checked out to a completely different machine. The "default" setting works. But the -Dspark2 is having a strange Spark versioning issue. I can not see how it were an issue of the CaffeOnSpark code. But in any case I am working to figure it out. Here is the error: 2016-06-19 20:57:31,141 [ScalaTest-main-running-DataFrameTest] INFO handler.ContextHandler - Started o.s.j.s.ServletContextHandler@546ed2a0{/static/sql,null,AVAILABLE} Notice the errors appear to reflect internal consistency issues within the Spark SQL classes themselves. I don't know how that could happen. I am presently rebuilding spark to try to shed some light. |
|
OK after rebuilding / re-installing spark-2.0.0-SNAPSHOT the above error went away. BOTH spark 2.0.0 and 1.6.0 were retested from scratch on this OTHER machine and work fine. INFO spark.SparkContext - Running Spark version 2.0.0-SNAPSHOT mvn clean package test |
|
I CANNOT REPRODUCE your issue. The tests are running fine on both 1.6.0 and 2.0.0-SNAPSHOT. The steps I used were shown above: repeating here for convenience. git clone --recursive https://github.com/javadba/CaffeOnSpark caos cd caffe-grid They both pass . |
|
ok I will checkout. When you get a chance merge locally in the CaffeOnSpark On Sun, Jun 19, 2016 at 9:35 PM, StephenBoesch notifications@github.com
|
|
OK. I did as requested: clone'd yahoo/master and locally merged the changes from my sparkv2_v1compatible branch. IT WORKS. git remote add javadba https://github.com/javadba/CaffeOnSpark git clone --recursive https://github.com/yahoo/CaffeOnSpark caos.master then do the normal build steps..now let's set up to rebase from my PRgit checkout -b merge_spark2_on_master 2016-06-19 22:03:07,394 [ScalaTest-main-running-DiscoverySuite] INFO spark.SparkContext - Running Spark version 1.6.02016-06-19 22:10:46,348 [sparkDriverActorSystem-akka.actor.default-dispatcher-4] INFO remote.RemoteActorRefProvider$RemotingTerminator - Remoting shut down. mvn -Dspark2 clean package 2016-06-19 22:24:15,967 [ScalaTest-main-running-DiscoverySuite] INFO spark.SparkContext - Successfully stopped SparkContext |
|
Cool. I was able to run you fork with spark. Will try with spark2 in the On Sun, Jun 19, 2016 at 10:25 PM, StephenBoesch notifications@github.com
|
|
I followed all the above steps which you mentioned. Everything went fine 2016-06-19 23:36:19,651 [dag-scheduler-event-loop] INFO On Sun, Jun 19, 2016 at 10:49 PM, Mridul Jain jain.mridul@gmail.com wrote:
|
|
Is "make build" different than "make; cd caffe-grid; mvn clean package" ? 2016-06-19 23:09 GMT-07:00 mriduljain notifications@github.com:
|
|
"make build" and "make buildosx" completely do not work for me. Errors Would you please provide the specific steps you used. 2016-06-19 23:12 GMT-07:00 Stephen Boesch javadba@gmail.com:
|
|
it is: On Sun, Jun 19, 2016 at 11:12 PM, StephenBoesch notifications@github.com
|
|
it's in the Makefile of root directory On Sun, Jun 19, 2016 at 11:14 PM, Mridul Jain jain.mridul@gmail.com wrote:
|
|
Apologies I had been briefly in an old/incorrect directory. Now after going back to the yahoo/master with rebase to make buildosx runs fine 2016-06-19 23:15 GMT-07:00 mriduljain notifications@github.com:
|
|
Can we get on a skype call? There is a nitty gritty detail of build env 2016-06-19 23:24 GMT-07:00 Stephen Boesch javadba@gmail.com:
|
|
I will checkout in morning and then we can decide what to do. May be a On Sun, Jun 19, 2016 at 11:25 PM, StephenBoesch notifications@github.com
|
|
sounds good. thx 2016-06-19 23:27 GMT-07:00 mriduljain notifications@github.com:
|
|
ok build and local test passed for both 1 and 2. For 2 I had to addthe following though to caffe-grid/pom.xml Now I am testing mnist but need to compile spark2 locally first. I am not sure may be that or the jar from maven directly might take care so only one of those options is required? Please document the exact steps for spark2 somewhere. Also default would be 1.6 for now, so when folks do make build, it will trigger on 1.6. For 2.0 please document the setup and steps. |
|
The documentation does already cover the case that Spark 2.X is in the Your question is maybe you're not used to doing mvn install from a 2016-06-20 21:04 GMT-07:00 mriduljain notifications@github.com:
|
|
I am able to do it; but look at the issues section which range from all On Mon, Jun 20, 2016 at 9:08 PM, StephenBoesch notifications@github.com
|
|
I have updated the PR with the details. 2016-06-20 21:08 GMT-07:00 Stephen Boesch javadba@gmail.com:
|
|
That's fair: please see if the updated README.md were sufficient. 2016-06-20 21:17 GMT-07:00 mriduljain notifications@github.com:
|
|
@anfeng Please review it while I test mnist |
| Then: update the CaffeOnSpark caffe-grid/pom.xml in the "spark2" profile section: set the correct spark (e.g. **2.0.0-SNAPSHOT**),scala (e.g. **2.11.7**)), and hadoop (e.g. **2.7.1**)) versions: | ||
|
|
||
| ``` | ||
| <profiles> |
There was a problem hiding this comment.
Why do we include pom.xml code in readme? Users should not need to touch pom, since you have already modified it.
There was a problem hiding this comment.
Please read : the updated README itself alread has the answer to your question:
set the correct spark (e.g. **2.0.0-SNAPSHOT**),scala (e.g. **2.11.7**)), and hadoop (e.g. **2.7.1**)) versions:
The user may need to update the version: after all they may have 2.0.0-preview not SNAPSHOT.
There was a problem hiding this comment.
We could tell them to adjust those entries, without duplicating pom segements
There was a problem hiding this comment.
I had already done so. But then @mriduljain did not even "notice" / see the instructions. These instructions make it more clear. If you feel strongly then bring it up with him since I added this based on his feedback.
There was a problem hiding this comment.
Oh based on your comment I did find a better approach: it looks like command line based System properties can be used.
So we could do this -Dspark.version=2.0.0-preview -Dhadoop.version=2.7.2 -Dscala.major.version=2.11 -Dscala.version=2.11.8.
I will test this out. The question is: can the -Pspark2 work in conjunction with the -D system properties .. or will the latter overrule/overwrite the former? I'm not sure.
If it does work then I will make this change to the README.md (NO change required to the pom.xml)
There was a problem hiding this comment.
Can't this be added to pom conditionally. Like we have build/buildosx which
do everything required
On Mon, Jun 20, 2016 at 10:13 PM, StephenBoesch notifications@github.com
wrote:
In README.md
#79 (comment):@@ -59,6 +59,40 @@ Please note:
- Batch sizes specified in prototxt files are per device.
- Memory layers should not be shared among GPUs, and thus "share_in_parallel: false" is required for layer configuration.
+## Building for Spark 2.X
+To Build for Spark 2.X please include
+
+* mvn -Dspark2
+
+Spark2 Pre-release note You will need to do the following:
+- in the Spark git clone :
+
+- mvn install
+
+Then: update the CaffeOnSpark caffe-grid/pom.xml in the "spark2" profile section: set the correct spark (e.g. 2.0.0-SNAPSHOT),scala (e.g. 2.11.7)), and hadoop (e.g. 2.7.1)) versions:
+
+```
I had already done so. But then @mriduljain
https://github.com/mriduljain did not even "notice" / see the
instructions. These instructions make it more clear. If you feel strongly
then bring it up with him since I added this based on his feedback.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/yahoo/CaffeOnSpark/pull/79/files/0a3a9daf07ae0b44a3f2e63a272f631b1130f937#r67809032,
or mute the thread
https://github.com/notifications/unsubscribe/ACCTVbhB1x5tnhKgH9ty1mdlfQ5VMkhEks5qN3L0gaJpZM4IxfyE
.
There was a problem hiding this comment.
cool the lower the number of steps, the better..specifically which avoid users to edit files.
There was a problem hiding this comment.
So I have updated the pom.xml Here is the updated info
…rol Spark, scala and hadoop versions
|
Made some further tweaks to the README.md and successfully re-tested via cut-and-paste directly from the updated instructions. |
|
@javadba I think that we are almost ready to merge this pull request. I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄 |
|
I did not know about it - and had asked earlier based on one of the 2016-06-21 21:05 GMT-07:00 anfeng notifications@github.com:
|
|
I am having some difficulty to follow/track the comments. Is there a way to see "all unresponded to comments" ? |
|
I missed this comment - sorry . Is there a way on the github GUI to see 2016-06-21 16:23 GMT-07:00 anfeng notifications@github.com:
|
|
CLA signed: Good to see you, StephenBoesch You previously signed this CLA on: |
|
+1 |
…ile switch