And if we find incorrect declaration, we fix it, not simply assuming many of them also has problem and we cannot rely on them - otherwise the type-safe APIs in Scala also does not make sense. On Sat, Sep 29, 2018 at 7:10 PM YiZhi Liu <eazhi....@gmail.com> wrote: > > It also makes sense to me if we have it under namespace NDArray, not > creating new JavaNDArray. But again, uniform experience is important. > > What I responded is your comment "keep scala macros minimum", I don't > think "scala macro" equals "cryptic code". Even though it does, what > we need to do is to find an alternative way to do code generation, not > making code generation minimum. > > Since you agree to have 30+ operators have Builder, what prevents from > having all of them have Builder? > - They're auto-generated, the auto-generation "cryptic" code is anyway > there. And "two different paths of code" (though I don't totally > agree) is anyway there. > - What else? 200+ classes is a very tiny increasing in file size > (~3MB) compare to current status. And won't have any performance issue > on modern JVM. > > Just remind, technical discussion is not about who gives who a lecture. > On Sat, Sep 29, 2018 at 6:41 PM Naveen Swamy <mnnav...@gmail.com> wrote: > > > > Well, I am not sure(I don't think) we need Builder for every API in > > NDArray. For APIs that take long list of parameters, I agree to add Builder. > > Look at the API distribution based on number of arguments here: > > https://gist.github.com/nswamy/2dea72e514cc7bfc675f68aef9fe78bb > > about 30 APIs have 7 or more arguments.. I agree to add Builders for these > > APIs not separately but to the existing Scala APIs but not separately only > > for Java. > > APIs sorted by number of arguments is here, take a look : > > https://gist.github.com/nswamy/e941cb94658b3960eec40bf00b970ac5 > > > > Many of the arguments i think are actually mandatory but incorrectly > > declared optional on the backend, for example look at SwapAxis > > "def SwapAxis (data : NDArray, dim1 : Option[Int] = None, dim2 : > > Option[Int] = None, out : Option[NDArray] = None) : NDArrayFuncReturn" > > Why is dim1 and dim2 Optional, this is an error in the declaration on the > > backend, I think there might be many of these? > > > > My answers to your other responses are below inline: > > > > On Sat, Sep 29, 2018 at 3:37 PM YiZhi Liu <eazhi....@gmail.com> wrote: > > > > > Some of my comments inline: > > > > > > > Why can we not create the builder just for these APIs( which we > > > discussed), why is it necessary to add 200 Apis > > > It is about unified user-experience. And we get rid of annoying extra > > > "out=null" in every operator. > > > > > > > Are you suggesting to create builder for each and every API? > > > Only for those are necessary. For NDArray.XXX, yes. > > > > > I think this is a ridiculous list of Builders, I think we can keep the > > 'out' parameter > > > > > 1) The NDArray APIs in question are not following functional style of > > > programming, in fact they are just static methods defined on an > > > NDArray object - so Scala users are not losing much by using null in > > > place of None. > > > You can create a implicit to maintain backward compatibility > > > - I doubt implicit can work in such case from None -> null. > > > > > > > It is just writing getOrElse in your implicit, so it will work. > > scala> implicit def optionStringToString(a: Option[String]): String = { > > | a.getOrElse(null) > > | } > > > > 2) It is adding 220+ APIs(I understand it is generated) for NDArray alone > > > - As I explained how it can improve user experiences > > > > > I don't think we need to write builders for 221 APIs we have, may be for 30 > > or so. Uniform experience is good goal but it also has to be practical and > > make sense. > > > > 3) this is adding another 100s of APIs unnecessarily, we are starting with > > > NDArray but we can't stop there, we will have to do this for Symbol, > > > Executor, Iterators, etc., . > > > - This is a good point, actually I prefer not to make JavaExecutor, > > > JavaIterators > > > > > What I was aiming is also users have the same experience across Interfaces > > - now you are forgoing uniform experience, so like you said its all > > trade-off and a good trade-off doesn't cause too much overhead/ > > > > > > > 4) I don't want to be fixing bugs and maintaining code in 2 places. > > > - Type-safe parsing is shared. I think Qing is more qualified to comment. > > > > It creates two different paths of code for Scala and Java - how is it going > > to be shared. I am afraid we are going to make it more complicated than > > necessary by duplicating code. > > > > > > > > > 5) I want the cryptic code(# scala macros) to a minimum. > > > - MXNet decides to do operator generation in frontend bindings. It's > > > the developers' responsibility to understand the techniques they are > > > using. Maybe not a so proper analogy - "I don't know RL / RL is hard > > > to tune / ..." is not a reason for "I want to keep RL implementation > > > in MXNet as a small part as possible" > > > > > > Now, this is a response I don't like. I don't know where you were going > > with your analogy but know that it sounds condescending - I am going to > > ignore(assuming good intentions) that and explain what I mean. > > I here is I the developer/user who deploys MXNet code in production and > > have to deal with the aftermath myself not you(MXNet developers). > > From your comment it occurs to me you probably have never been on > > pager-duty. I have been on pager-duty both for the code I wrote and those > > that was written by others and thrown over the fence. > > If you get woken up by a beep at the middle of the night, that is not the > > time to prove your intelligence. Its time to mitigate the issue asap for > > that your code needs to be easy to follow, should follow well defined > > patterns, etc., -- i don't need to give you a lecture. > > IMHO It is extremely important for frameworks like Apache MXNet which are > > used by others for their production to keep code simple and *cryptic code > > to a minimum* and yes you(we - MXNet developers) are not answering the > > beepers when your(MXNet) users deploy their code in their production so > > make their life simple. > > > > 6) increased compilation time & bad developer experience - the time to > > > compile has gone up quite a bit since we added the APIs last release on my > > > 3 year old laptop already.. I think adding 400+ APIs unnecessarily would > > > significantly increase build time and bad developer experience > > > - I don't think increasing such a bit compilation time is a problem > > > compared to bad user experience. > > > > I am not suggesting bad user experience but to take a practical approach - > > having a bad developer experience is not great either. > > > > > > > > > > 7) I want to keep the core of the framework to be in Scala - because it > > > allows you to write concise code - Yes it has a bit of learning curve, not > > > everyone needs to know. I would rather invest in solidifying the Scala > > > APIs > > > and add more features in Scala(RNN, Support GluonHybridizedBlock...there > > > is > > > quite bit of work ) - do you want to rewrite everything in Scala and Java. > > > - I agree with "don't rewrite everything in Scala and Java", IMO > > > JavaNDArray is the only one good to have. JShape, JContext, etc. are > > > not so necessary. > > > > > > Either you go all Java or make accommodation in Scala code to work for > > APIs so your users know what to expect(uniform experience across). > > > > > 8) Also, the discussion is not creating NDArray class for Java, just > > > generate certain APIs to cater for Java incompatibility. > > > - Yes I agree it's about "generate certain APIs to cater for Java > > > incompatibility", though I think NDArray.api.XXX does not meet Java > > > users' demands. > > > > > On Sat, Sep 29, 2018 at 12:05 PM Naveen Swamy <mnnav...@gmail.com> wrote: > > > > > > > > I know it is about trade-off. I am suggesting a trade-off , how many > > > apis do we have that takes too many parameters ? > > > > From what I recall its around 20. Why can we not create the builder just > > > for these APIs( which we discussed), why is it necessary to add 200 Apis ? > > > > Are you suggesting to create builder for each and every API? > > > > > > > > I disagree with your opinion that they are not important and would like > > > to hear from others. > > > > > > > > I am curious to see how the #2 looks like compared to #1 > > > > Andrew/Qing, can you paste the generated Apis that you have for both > > > Scala and Java in a gist please. > > > > > > > > > On Sep 29, 2018, at 2:41 PM, YiZhi Liu <eazhi....@gmail.com> wrote: > > > > > > > > > > Naveen, software designing is all about tradeoff, every feature we > > > > > introduce causes more compiling time, more efforts to maintain, etc. > > > > > > > > > > The main difference is. > > > > > > > > > > Option #1: Java users do > > > > > NDArray.BatchNorm(data, gamma, beta, null, null, null, null, null, > > > > > null, null, null, null, null, null); > > > > > (and because every operator has an argument "out", users need to add > > > > > an extra "null" to the function call almost every time.) > > > > > > > > > > Option #2, Java users do > > > > > JavaNDArray.BatchNorm(data).setGamma(gamma).setBeta(beta).invoke(); > > > > > > > > > > I don't think any of the reasons you listed is so important as the > > > > > benefit above we got from option #2. > > > > >> On Sat, Sep 29, 2018 at 8:24 AM Naveen Swamy <mnnav...@gmail.com> > > > wrote: > > > > >> > > > > >> Java APIs are not like Clojure - The current proposal is only to > > > build a > > > > >> few thin wrappers for Inference. > > > > >> > > > > >> To better represent the two cases and this discussion in particular, > > > here > > > > >> is an example API > > > > >> > > > > >> 1) def Activation (data : org.apache.mxnet.NDArray, act_type : > > > String, out > > > > >> : Option[NDArray] = None) : org.apache.mxnet.NDArrayFuncReturn > > > > >> or > > > > >> 2) def Activation (data : org.apache.mxnet.NDArray, act_type : > > > String, out > > > > >> : NDArray) : org.apache.mxnet.NDArrayFuncReturn > > > > >> > > > > >> The discussion is should we add(generate) 200+ APIs to make it Java > > > > >> compatible, ie., remove the Option class and the None default value > > > which > > > > >> Java does not understand from Option 1) > > > > >> > > > > >> my suggestion was to remove the Option class and create a implicit > > > > >> for > > > > >> backward compatibility and use null instead of None, Andrew and I > > > disagreed > > > > >> on this, so I suggested to raise a discussion on dev@ to get more > > > opinions > > > > >> and one of us will disagree and commit. Thanks for raising it :) > > > > >> > > > > >> | * def Activation (data : org.apache.mxnet.NDArray, act_type : > > > String, out > > > > >> : NDArray = null) : org.apache.mxnet.NDArrayFuncReturn | > > > > >> -- > > > > >> > > > > >> 1) It is not true that Scala users will lose *default/optional* > > > arguments - > > > > >> if we followed the above, they will use null or None, though I do not > > > like > > > > >> using nulls, this is a fine compromise. > > > > >> To keep backward compatibility we can create a implicit to convert > > > > >> Option.None to nulls and Option.Some-> Option.get(), so you are not > > > going > > > > >> to break users who might have been using the APIs that were released > > > in > > > > >> 1.3. The current incompatibility is only this w.r.t. NDArrays. > > > > >> > > > > >> 2) Now about the Scala Macros - they are not simple to read or use, > > > When I > > > > >> and Qing started working on the #Scala Macros to improve the APIs, it > > > took > > > > >> us a good amount of time to get a hang of it. I don't want to add > > > > >> additional code when not necessary. > > > > >> > > > > >> My suggestion and vote is to modify existing Macro(i.e., #1 from the > > > > >> original email with the necessary clarification above) and make it > > > > >> compatible with Java > > > > >> Here are my reasons > > > > >> 1) The NDArray APIs in question are not following functional style of > > > > >> programming, in fact they are just static methods defined on an > > > NDArray > > > > >> object - so Scala users are not losing much by using null in place of > > > None. > > > > >> You can create a implicit to maintain backward compatibility > > > > >> 2) It is adding 220+ APIs(I understand it is generated) for NDArray > > > alone > > > > >> 3) this is adding another 100s of APIs unnecessarily, we are starting > > > with > > > > >> NDArray but we can't stop there, we will have to do this for Symbol, > > > > >> Executor, Iterators, etc., . > > > > >> 3) I don't want to be fixing bugs and maintaining code in 2 places. > > > > >> 4) I want the cryptic code(# scala macros) to a minimum. > > > > >> 5) increased compilation time & bad developer experience - the time > > > > >> to > > > > >> compile has gone up quite a bit since we added the APIs last release > > > on my > > > > >> 3 year old laptop already.. I think adding 400+ APIs unnecessarily > > > would > > > > >> significantly increase build time and bad developer experience > > > > >> 6) I want to keep the core of the framework to be in Scala - because > > > it > > > > >> allows you to write concise code - Yes it has a bit of learning > > > curve, not > > > > >> everyone needs to know. I would rather invest in solidifying the > > > Scala APIs > > > > >> and add more features in Scala(RNN, Support > > > GluonHybridizedBlock...there is > > > > >> quite bit of work ) - do you want to rewrite everything in Scala and > > > Java. > > > > >> 7) Also, the discussion is not creating NDArray class for Java, just > > > > >> generate certain APIs to cater for Java incompatibility. > > > > >> > > > > >> @Andrew: To your response to Qing's comments - you cannot just > > > consider it > > > > >> as just generating NDArray's APIs and instead I suggest to take a > > > wholistic > > > > >> view of all the various implications. > > > > >> > > > > >> @Chris: Yes, Scala has a bit of learning curve - the goal is not > > > having > > > > >> every developer to deal with how these APIs are generated, > > > > >> the problem exists either ways with the above proposal. I might agree > > > if we > > > > >> were to move away completely(with a thorough discussion and valid > > > reasons) > > > > >> and instead use AspectJ or similar to write these APIs, the > > > discussion is > > > > >> about using Scala Macros to generate 2 different types of APIs which > > > are > > > > >> functionally not different and usability wise are very very similar, > > > look > > > > >> at the example. > > > > >> Thanks for your input, I will deposit your 0.02$ in our JIRA bank :) > > > > >> > > > > >> @Carin: It requires more effort to use AspectJ or similar to generate > > > APIs > > > > >> using reflection or at compile time, here we need to generate at > > > compile > > > > >> time so Java users have the API signature on their IDEs. > > > > >> > > > > >> Thanks, Naveen > > > > >> > > > > >> P.S: I am traveling and my responses will be delayed. > > > > >> > > > > >> > > > > >>> On Fri, Sep 28, 2018 at 10:25 AM Carin Meier <carinme...@gmail.com> > > > wrote: > > > > >>> > > > > >>> Sorry bad paste on the gist - here is the good one > > > > >>> https://gist.github.com/gigasquid/01cd48f563db4739910592dd9ac9db20 > > > > >>> > > > > >>>> On Fri, Sep 28, 2018 at 10:24 AM Carin Meier <carinme...@gmail.com> > > > wrote: > > > > >>>> > > > > >>>> +1 on option #2 > > > > >>>> > > > > >>>> In the case of minimizing the the overhead for code maintenance, I > > > wanted > > > > >>>> to suggest the option of investigating generating code from the > > > > >>>> Java > > > > >>>> Reflection for the Java APIs. I did a quick gist from Clojure of > > > what > > > > >>> the > > > > >>>> generated classes look like from the current Scala Symbol.api for > > > > >>>> FullyConnected here > > > > >>>> https://gist.github.com/gigasquid/01cd48f563db4739910592 > > > > >>>> > > > > >>>> I looks like that there is always a base Java class generated will > > > all > > > > >>> the > > > > >>>> arguments. If this is the case, then there is a possibility to > > > generate a > > > > >>>> Java api based on this Java method automatically with just a > > > conversion > > > > >>> for > > > > >>>> the Scala option and it might be reusable for all the packages. > > > > >>>> > > > > >>>> Not sure if it will work for this use case, but thought I would > > > bring it > > > > >>>> up in case it's helpful. > > > > >>>> > > > > >>>> - Carin > > > > >>>> > > > > >>>> On Fri, Sep 28, 2018 at 7:05 AM Davydenko, Denis > > > <d...@amazon.com.invalid > > > > >>>> > > > > >>>> wrote: > > > > >>>> > > > > >>>>> +1 on option #2. Having clear Java interface for NDArray, from my > > > > >>>>> perspective, would be a better experience for Java users as it > > > won't > > > > >>>>> require them to deal with Scala code in any capacity. Overhead of > > > extra > > > > >>>>> code for additional macros is justified, in my mind, as it will be > > > > >>>>> introduced with option #1 either way, just in a different place. > > > > >>>>> > > > > >>>>> -- > > > > >>>>> Thanks, > > > > >>>>> Denis > > > > >>>>> > > > > >>>>> On 9/27/18, 6:14 PM, "YiZhi Liu" <eazhi....@gmail.com> wrote: > > > > >>>>> > > > > >>>>> I vote for "2.) Leave the existing macro in place and add another > > > > >>>>> which generates a Java friendly version" > > > > >>>>> > > > > >>>>> @Qing @Andrew, could you give some examples, so that people can > > > > >>> better > > > > >>>>> understand how it provides "best possible experience" to Java > > > users. > > > > >>>>> > > > > >>>>> I have no strong preference between having JavaShape & > > > > >>>>> JavaContext > > > > >>> or > > > > >>>>> not. > > > > >>>>> On Thu, Sep 27, 2018 at 5:56 PM Andrew Ayres < > > > > >>>>> andrew.f.ay...@gmail.com> wrote: > > > > >>>>>> > > > > >>>>>> That's not really the conversation I'm wanting to have. I want a > > > > >>>>> discussion > > > > >>>>>> about the macros with respect to NDArray so that we can get > > > > >>>>> agreement on > > > > >>>>>> our path forward with respect to implementing the NDArray > > > > >>>>>> wrapper. > > > > >>>>>> > > > > >>>>>> The design that was put forth and agreed to was for a a Java > > > > >>>>> wrapper around > > > > >>>>>> the Scala API. Adding a bunch of Java friendly methods inside the > > > > >>>>> Scala > > > > >>>>>> code would create a mess for users. Maintenance would be > > > > >>>>> essentially the > > > > >>>>>> same for both because either way you're going to be updating Java > > > > >>>>> methods > > > > >>>>>> when you make Scala changes. > > > > >>>>>> > > > > >>>>>> Let's please stick with the issue in the original email. > > > > >>>>>> > > > > >>>>>> Thanks, > > > > >>>>>> Andrew > > > > >>>>>> > > > > >>>>>>> On Thu, Sep 27, 2018 at 5:22 PM Qing Lan <lanking...@live.com> > > > > >>>>>> wrote: > > > > >>>>>> > > > > >>>>>>> I would like to loop this back a layer. Current, there is a > > > > >>>>> discussion in > > > > >>>>>>> the MXNet Scala community on the ways to implement the Java > > > > >>> APIs. > > > > >>>>> Currently > > > > >>>>>>> there are two thoughts: > > > > >>>>>>> > > > > >>>>>>> 1. Make Scala Java Friendly (Create Java compatible methods in > > > > >>>>> the Scala > > > > >>>>>>> Class. such as NDArray with Java compatible constructor) > > > > >>>>>>> 2. Make Java friendly wrappers in Scala (Andrew's explanation > > > > >>>>> below) > > > > >>>>>>> > > > > >>>>>>> The first approach require minimum input from our side to > > > > >>>>> implement > > > > >>>>>>> however bring user a bunch of useless api they may not want to > > > > >>>>> use. It also > > > > >>>>>>> makes Scala package heavier. The good thing is these two > > > > >>> packages > > > > >>>>> require > > > > >>>>>>> minimum maintenance cost. As a tradeoff, if any time in the > > > > >>>>> future we want > > > > >>>>>>> to make Java big (make Java as the primary language supported by > > > > >>>>> MXNet), > > > > >>>>>>> then the migration from Scala to Java will be harmful. Spark > > > > >>>>> consider this > > > > >>>>>>> carefully and decide not to change much on their Scala code base > > > > >>>>> to make it > > > > >>>>>>> more Java. > > > > >>>>>>> > > > > >>>>>>> The second approach will make unique NDArray, Shape, Context and > > > > >>>>> more. The > > > > >>>>>>> good thing about this is we can always holds a version control > > > > >>> on > > > > >>>>> Java. > > > > >>>>>>> Some breaking changes on Scala may not influence much on Java. > > > > >>> It > > > > >>>>> did the > > > > >>>>>>> best way to decouple the module and good for us to build unique > > > > >>>>> pipeline > > > > >>>>>>> for Java. The bad thing with this design is the maintenance cost > > > > >>>>> as we need > > > > >>>>>>> to keep two code bases, but it also make Java side easy to > > > > >>> change > > > > >>>>> to make > > > > >>>>>>> it better compatible with users. > > > > >>>>>>> > > > > >>>>>>> Thanks, > > > > >>>>>>> Qing > > > > >>>>>>> > > > > >>>>>>> On 9/27/18, 3:25 PM, "Andrew Ayres" <andrew.f.ay...@gmail.com> > > > > >>>>> wrote: > > > > >>>>>>> > > > > >>>>>>> Hi, > > > > >>>>>>> > > > > >>>>>>> Currently, we're working to implement a new Java API and > > > > >>>>> would like > > > > >>>>>>> some > > > > >>>>>>> feedback from the community on an implementation detail. In > > > > >>>>> short, the > > > > >>>>>>> new > > > > >>>>>>> Java API will use the existing Scala API (in a manner > > > > >>> similar > > > > >>>>> to how > > > > >>>>>>> the > > > > >>>>>>> current Clojure API works). This basically means that we're > > > > >>>>> making Java > > > > >>>>>>> friendly wrappers to call the existing Scala API. > > > > >>>>>>> > > > > >>>>>>> The feedback we're looking for is on the implementation of > > > > >>>>> NDArray. > > > > >>>>>>> Scala's > > > > >>>>>>> NDArray has a significant amount of code which is generated > > > > >>>>> via macros > > > > >>>>>>> and > > > > >>>>>>> we've got two viable paths to move forward: > > > > >>>>>>> > > > > >>>>>>> 1.) Change the macro to generate Java friendly methods - To > > > > >>>>> do this > > > > >>>>>>> we'll > > > > >>>>>>> modify the macro so that the generated methods won't have > > > > >>>>>>> default/optional > > > > >>>>>>> arguments. There may also have to be some changes to > > > > >>>>> parameter types to > > > > >>>>>>> make them Java friendly. The big advantage here is that > > > > >>>>> ongoing > > > > >>>>>>> maintenance > > > > >>>>>>> will easier. The disadvantages are that we'll be changing > > > > >>> the > > > > >>>>> existing > > > > >>>>>>> Scala NDArray Infer API (it's marked experimental) and Scala > > > > >>>>> users will > > > > >>>>>>> lose the ability to use the default and optional arguments. > > > > >>>>>>> > > > > >>>>>>> 2.) Leave the existing macro in place and add another which > > > > >>>>> generates a > > > > >>>>>>> Java friendly version - The biggest issue here is that we'll > > > > >>>>> be > > > > >>>>>>> doubling > > > > >>>>>>> the number of macros that we've got to maintain. It'll > > > > >>> become > > > > >>>>> even more > > > > >>>>>>> overhead once we start expanding the Java API with more > > > > >>>>> classes that > > > > >>>>>>> use > > > > >>>>>>> generated code like this. The advantages are that the > > > > >>>>> existing Scala > > > > >>>>>>> NDArray Infer API would remain unchanged for Scala users and > > > > >>>>> that the > > > > >>>>>>> new > > > > >>>>>>> macro could be optimized to give the best possible > > > > >>> experience > > > > >>>>> to the > > > > >>>>>>> Java > > > > >>>>>>> API. > > > > >>>>>>> > > > > >>>>>>> Thanks, > > > > >>>>>>> Andrew > > > > >>>>> > > > > >>>>> > > > > >>>>> > > > > >>>>> -- > > > > >>>>> Yizhi Liu > > > > >>>>> DMLC member > > > > >>>>> Amazon Web Services > > > > >>>>> Vancouver, Canada > > > > > > > > > > > > > > > > > > > > -- > > > > > Yizhi Liu > > > > > DMLC member > > > > > Amazon Web Services > > > > > Vancouver, Canada > > > > > > > > > > > > -- > > > Yizhi Liu > > > DMLC member > > > Amazon Web Services > > > Vancouver, Canada > > > > > > > > > > -- > Yizhi Liu > DMLC member > Amazon Web Services > Vancouver, Canada
-- Yizhi Liu DMLC member Amazon Web Services Vancouver, Canada
