Hi Qing I didn't know about the type parser, I had a look at the code. Nice idea !
Pedro On Wed, May 2, 2018 at 8:57 PM, Qing Lan <lanking...@live.com> wrote: > Hi Pedro, > > Thanks for your comments! > > For the first one: please see the definition for attr here: > https://mxnet.incubator.apache.org/api/python/symbol/ > symbol.html?highlight=attr#mxnet.symbol.Symbol.attr. It is a user defined > Struct. Our old API indeed using null instead of Option + None > configuration and the new API will go Option + None. > > For Second one, the new API provide the type for attribute: > e.g: def Activation(data : Option[Symbol], act_type : String, name : > Option[String], attr : Option[Map[String, String]]) > def Pad(data : Option[Symbol] = None, mode : String, pad_width : Shape, > constant_value : Option[Double] = None, name : Option[String] = None, attr > : Option[Map[String, String]] = None) > And you are right, that's exactly we are working on now, make a parser for > Scala to C++ Type, see here: > https://github.com/lanking520/incubator-mxnet/blob/scala- > macros-impl/scala-package/macros/src/main/scala/org/ > apache/mxnet/SymbolMacro.scala#L147 > > Thanks, > Qing > > On 5/2/18, 3:50 AM, "Pedro Larroy" <pedro.larroy.li...@gmail.com> wrote: > > Hi > > I had a brief look and I have some comments: > > 1 - Excessive use of Option: Having an optional map is often not > necessary. > What's the semantic difference between an empty map and passing None? > What > about a variable argument list of Pairs like: > > (attr: Pair[String,Sring]*) it can be then called like ("lr" -> > "0.01", > "activation" -> "relu") with variable number of arguments. > > null is definitely bad, but we can have an empty map or other default > arguments that make sense in some cases, like an empty string for name > instead of an optional string. I'm very much pro Option instead of > null but > it adds clunkiness that is not always necessary, as explained. > > 2 - One of the fundamental problems is that the arguments are untyped, > of > type String -> String, I don't see this solved in this proposal. Is it > possible to do something better, ie. have typed arguments? when we > extend > dmlc::Parameter we actually know the C++ type, we could do something > smart > about this. For example using clang to extract the types and generate > the > API with proper types. > > > Pedro. > > On Wed, May 2, 2018 at 4:01 AM, Naveen Swamy <mnnav...@gmail.com> > wrote: > > > No, the implementation still comes from MXNet backend. > > I don't think there would be any overhead unlike C++ with virtual > > functions, Interface or trait is just telling you what the signature > looks > > like. users program against interfaces and can choose a different > > implementation if they like. For example(only to explain the topic) > if we > > had different implementations for CPU and GPU, they could simply > choose the > > version of Implementation they like. To visualize think of a bridge > design > > pattern. > > > > > > > > On Tue, May 1, 2018 at 6:49 PM, Marco de Abreu < > > marco.g.ab...@googlemail.com > > > wrote: > > > > > Hey Naveen, > > > > > > I'm not familiar with Scala, so please correct me if I'm wrong > with my > > > assumptions. This basically sounds like we'd have an interface for > the > > > Scala API and a separate implementation, right? > > > > > > In general, I really like these decoupled approaches (you already > > > highlighted the advantages)! It would introduce a small overhead > since > > we'd > > > have to define the traits and implementation at two (different) > > locations, > > > but it definitely increases clarity and separates concerns. I > don't think > > > we will have multiple implementations in future, but this might > allow us > > to > > > define the user-facing API in form of traits while allowing us to > change > > > the underlying "glue" as we want. > > > > > > My only slight concern is that this would introduce a bit more > overhead > > > since the documentation and API are defined somewhere else, but I > have no > > > experience with Scala, so I'll leave this up to the people who are > more > > > familiar with the topic. > > > > > > Best regards, > > > Marco > > > > > > On Tue, May 1, 2018 at 11:48 PM, Naveen Swamy <mnnav...@gmail.com> > > wrote: > > > > > > > I think this project is going to significantly improve the > usability of > > > > MXNet-Scala APIs. > > > > > > > > I had a offline discussion with Qing about this. I agree with > Yizhi > > that > > > > keeping in the same namespace will make it easy for existing and > new > > > users > > > > to quickly lookup the APIs. Along with that I have one > recommendation, > > > > first generate a trait for all the APIs and have the the > instance in > > the > > > > existing symbol. The usage would be something like below: > > > > > > > > trait SymbolAPIBase { > > > > /** > > > > api doc > > > > */ > > > > def Activation() > > > > } > > > > > > > > object SymbolAPI extends SymbolAPIBase { > > > > def Activation() > > > > } > > > > > > > > object Symbol { > > > > val api: SymbolBase = new SymbolAPI() > > > > } > > > > > > > > Now users would use it as > > > > Symbol.api.Activation() > > > > > > > > Creating a trait/Interface as several advantages: > > > > 1) User do not have to know which concrete class to use, can be > decided > > > > later(Dependency Injection or Configuration driven application) > > > > 2) Easier to change the implementation later without breaking > the user > > > code > > > > -> You can insert different implementations through > configuration using > > > > frameworks such as Spring, etc., -> This is standard in most JVM > driven > > > > projects > > > > 3) Easier to Unit test -> You can create Mocks easily using > Mockito > > > > 4) helps with multiple inheritance…you cannot inherit multiple > classes > > > > > > > > Let me know what do you guys think. > > > > > > > > Thanks, Naveen > > > > > > > > On Sun, Apr 22, 2018 at 9:09 PM, YiZhi Liu <liuyi...@apache.org> > > wrote: > > > > > > > > > I personally like the design here. Since we have seen technical > > > > > difficulties of compatibility, I would like to ask people pay > > > > > attention to the 'How to combine with existing APIs' section: > > > > > https://cwiki.apache.org/confluence/display/MXNET/ > > > > > MXNet+Scala+API+Usability+Improvement# > MXNetScalaAPIUsabilityImprovem > > > ent- > > > > > HowtocombinewithexistingAPIs > > > > > > > > > > Qing proposed three options, > > > > > > > > > > 1. Add a new Class/Object called "NewSymbol/NDArray" with full > > > > > implementation. > > > > > 2. Create a new Class and change the name space for all of the > > > > > functions (e.g Activation -> NewActivation) and let > Symbol/NDArray > > > > > extends that. > > > > > 3. Create a new Class and override the Same functions with > different > > > > > implementations. > > > > > > > > > > If we have to choose from option 1 and 2, I would like to +0.5 > for > > > > > option 2, with which users can quickly aware of the new > easy-to-use > > > > > API: they type 'Symbol.' in IDE as usual and these functions > pop up. > > > > > > > > > > 2018-04-19 10:58 GMT-07:00 Qing Lan <lanking...@live.com>: > > > > > > Hi All, > > > > > > > > > > > > I am Qing, one of the Scala API maintainer for MXNet. I > would like > > to > > > > > propose a new design on Scala APIs, it will be really helpful > for > > user > > > to > > > > > use MXNet Symbol/NDArray. This is a follow-up from Naveen’s > proposal. > > > > > > > > > > > > Background: > > > > > > The current design on Scala would take arguments as > key-value pair > > > and > > > > > didn’t provide the type information for different arguments. > There > > are > > > > > document missing for different functions which makes it even > hard to > > > use. > > > > > > > > > > > > Our approach: > > > > > > We will provide a better designed Scala API for user to use > with > > full > > > > > documentation and arguments definition. All arguments will be > > > > specifically > > > > > targeted to different functions. Please see one example that > we show > > in > > > > the > > > > > Wiki<https://cwiki.apache.org/confluence/display/MXNET/ > > > > > MXNet+Scala+API+Usability+Improvement> and leave any thoughts > you > > may > > > > > have. This wiki includes examples, targets and scenarios we > have so > > > far. > > > > > > > > > > > > Thanks, > > > > > > Qing > > > > > > > > > > > > > > > > > > > > -- > > > > > Yizhi Liu > > > > > DMLC member > > > > > Amazon Web Services > > > > > Vancouver, Canada > > > > > > > > > > > > > > > > >
