Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[MXNET-357] New Scala API Design (NDArray) #10787

Merged
merged 5 commits into from
May 23, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Quick fix on redudant code
  • Loading branch information
lanking520 committed May 22, 2018
commit 846e88dffa2abfda9230b68c3b4dc575cf54b2ff
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ private[mxnet] object NDArrayMacro {
if (!NDArrayfunction.name.startsWith("_") || NDArrayfunction.name.startsWith("_contrib_")) {
Seq(
// scalastyle:off
// def transpose(kwargs: Map[String, Any] = null)(args: Any*)
// e.g def transpose(kwargs: Map[String, Any] = null)(args: Any*)
q"def $termName(kwargs: Map[String, Any] = null)(args: Any*) = {genericNDArrayFunctionInvoke($funcName, args, kwargs)}".asInstanceOf[DefDef],
// def transpose(args: Any*)
// e.g def transpose(args: Any*)
q"def $termName(args: Any*) = {genericNDArrayFunctionInvoke($funcName, args, null)}".asInstanceOf[DefDef]
// scalastyle:on
)
Expand Down Expand Up @@ -101,21 +101,6 @@ private[mxnet] object NDArrayMacro {

// Construct argument field
var argDef = ListBuffer[String]()
ndarrayfunction.listOfArgs.foreach(ndarrayarg => {
val currArgName = ndarrayarg.argName match {
case "var" => "vari"
case "type" => "typeOf"
case default => ndarrayarg.argName
}
if (ndarrayarg.isOptional) {
argDef += s"${currArgName} : Option[${ndarrayarg.argType}] = None"
}
else {
argDef += s"${currArgName} : ${ndarrayarg.argType}"
}
})
argDef += "name : String = null"
argDef += "attr : Map[String, String] = null"
// Construct Implementation field
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you expand the comment on why you are doing it here again
ie., you are still taking all the parameters and sending that across the JNI boundary as a map otherwise it would need to rewrite all JNI API implementation to match these. There is a small problem with this approach, we are not tying the type the backend expects from the Scala API. ie., we might by mistake generate a different type(if the mapping below is wrong)

var impl = ListBuffer[String]()
impl += "val map = scala.collection.mutable.Map[String, Any]()"
Expand All @@ -127,6 +112,12 @@ private[mxnet] object NDArrayMacro {
case "type" => "typeOf"
case default => ndarrayarg.argName
}
if (ndarrayarg.isOptional) {
argDef += s"${currArgName} : Option[${ndarrayarg.argType}] = None"
}
else {
argDef += s"${currArgName} : ${ndarrayarg.argType}"
}
var base = "map(\"" + ndarrayarg.argName + "\") = " + currArgName
if (ndarrayarg.isOptional) {
base = "if (!" + currArgName + ".isEmpty)" + base + ".get"
Expand Down