[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167598332
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
+
+}
+
+object MXNetHandlerType extends Enumeration {
+
+ type MXNetHandlerType = Value
+ val SingleThreadHandler = Value("MXNetSingleThreadHandler")
+ val OneThreadPerModelHandler = Value("MXNetOneThreadPerModelHandler")
+}
+
+class MXNetOneThreadPerModelHandler extends MXNetHandler {
+
+ private val threadFactory = new ThreadFactory {
+
+override def newThread(r: Runnable): Thread = new Thread(r) {
+ setName(classOf[MXNetOneThreadPerModelHandler].getCanonicalName)
+}
+ }
+
+ override val executor: ExecutorService = Executors.newFixedThreadPool(10,
threadFactory)
+
+ override def execute[T](f: => T): T = {
+val task = new Callable[T] {
+ override def call(): T = {
+// scalastyle:off println
+println("threadId: %s".format(Thread.currentThread().getId()))
+// scalastyle:on println
+f
+ }
+}
+val result = executor.submit(task)
+try {
+ result.get()
+}
+catch {
+ case e: ExecutionException => throw e.getCause()
Review comment:
@yzhliu What's the purpose of adding a `@throws` annotation? These are
normally disregarded in Scala.
It would make sense to document _why_ the executionexception is being
unwrapped here (the answer is "so it looks like the code was called inline").
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167596995
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
+
+}
+
+object MXNetHandlerType extends Enumeration {
+
+ type MXNetHandlerType = Value
+ val SingleThreadHandler = Value("MXNetSingleThreadHandler")
+ val OneThreadPerModelHandler = Value("MXNetOneThreadPerModelHandler")
+}
+
+class MXNetOneThreadPerModelHandler extends MXNetHandler {
+
+ private val threadFactory = new ThreadFactory {
+
+override def newThread(r: Runnable): Thread = new Thread(r) {
+ setName(classOf[MXNetOneThreadPerModelHandler].getCanonicalName)
Review comment:
The name here is a little confusing given that it is also used for
MXNetSingleThreadHandler.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167599469
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
+
+}
+
+object MXNetHandlerType extends Enumeration {
+
+ type MXNetHandlerType = Value
+ val SingleThreadHandler = Value("MXNetSingleThreadHandler")
+ val OneThreadPerModelHandler = Value("MXNetOneThreadPerModelHandler")
+}
+
+class MXNetOneThreadPerModelHandler extends MXNetHandler {
+
+ private val threadFactory = new ThreadFactory {
+
+override def newThread(r: Runnable): Thread = new Thread(r) {
+ setName(classOf[MXNetOneThreadPerModelHandler].getCanonicalName)
+}
+ }
+
+ override val executor: ExecutorService = Executors.newFixedThreadPool(10,
threadFactory)
Review comment:
A fixed threadpool of 10 does not enforce the invariant of having a single
thread for all MXNet use, which I believe to be the point of this approach. The
thread count here should be fixed to 1 (or use
`Executors.newSingleThreadExecutor`).
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167598960
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
+
+}
+
+object MXNetHandlerType extends Enumeration {
+
+ type MXNetHandlerType = Value
+ val SingleThreadHandler = Value("MXNetSingleThreadHandler")
+ val OneThreadPerModelHandler = Value("MXNetOneThreadPerModelHandler")
+}
+
+class MXNetOneThreadPerModelHandler extends MXNetHandler {
+
+ private val threadFactory = new ThreadFactory {
+
+override def newThread(r: Runnable): Thread = new Thread(r) {
+ setName(classOf[MXNetOneThreadPerModelHandler].getCanonicalName)
+}
+ }
+
+ override val executor: ExecutorService = Executors.newFixedThreadPool(10,
threadFactory)
+
+ override def execute[T](f: => T): T = {
Review comment:
Do you want to support the recursive case? If you submit a task which in
turn wants to submit a task to the MXNet thread, it will deadlock in this
arrangement. You can avoid this by checking whether you're already on the
managed thread and executing the code inline if so.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167600563
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/PredictBase.scala
##
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import ml.dmlc.mxnet.io.NDArrayIter
+import ml.dmlc.mxnet.{DataDesc, NDArray, Shape}
+import ml.dmlc.mxnet.module.Module
+
+import scala.collection.mutable.ListBuffer
+import org.slf4j.LoggerFactory
+
+/**
+ * Base Trait for MXNNet Predictor classes.
+ */
+trait PredictBase {
+
+ /**
+* This method will take input as IndexedSeq one dimensional arrays and
creates
+* NDArray needed for inference. The array will be reshaped based on the
input descriptors.
+* @param input: A IndexedSequence of Java one-dimensional array, An
IndexedSequence is
+* is needed when the model has more than one input/output
+* @return IndexedSequence array of outputs.
+*/
+ def predict(input: IndexedSeq[Array[Float]]): IndexedSeq[Array[Float]]
+
+ /**
+* Predict using NDArray as input. This method is useful when the input is
a batch of data
+* or when multiple operations on the input/output have to performed.
+* Note: User is responsible for managing allocation/deallocation of
NDArrays.
+* @param input: IndexedSequence NDArrays.
+* @return output of Predictions as NDArrays.
+*/
+ def predictWithNDArray(input: IndexedSeq[NDArray]): IndexedSeq[NDArray]
+
+}
+
+/**
+ * Implementation of predict routines.
+ *
+ * @param modelPathPrefix PathPrefix from where to load the model.
+ *Example: file://model-dir/resnet-152(containing
resnet-152-symbol.json,
+ *resnet-152-.params and optionally synset.txt).
+ *Supports model loading from various sources like
local disk,
+ *hdfs, https and s3. file://, hdfs://, https://,
s3://
+ * @param inputDescriptors Descriptors defining the input node names, shape,
+ * layout and Type parameters
+ * @param outputDescriptors Descriptors defining the output node names, shape,
+ * layout and Type parameters
+ */
+class Predictor(modelPathPrefix: String,
+ protected val inputDescriptors: IndexedSeq[DataDesc],
+ protected var outputDescriptors:
+ Option[IndexedSeq[DataDesc]] = None) extends PredictBase {
+
+ private val logger = LoggerFactory.getLogger(classOf[Predictor])
+
+ protected var batchIndex = inputDescriptors(0).layout.indexOf('N')
+ protected var batchSize = if (batchIndex != -1 )
inputDescriptors(0).shape(batchIndex) else 1
+
+ protected var iDescriptors = inputDescriptors
+
+ inputDescriptors.foreach((f: DataDesc) => require(f.layout.indexOf('N') ==
batchIndex,
+"batch size should be in the same index for all inputs"))
+
+
+ if (batchIndex != -1) {
+inputDescriptors.foreach((f: DataDesc) => require(f.shape(batchIndex) ==
batchSize,
+ "batch size should be same for all inputs"))
+ } else {
+// TODO: this is assuming that the input needs a batch
+iDescriptors = inputDescriptors.map((f : DataDesc) => new DataDesc(f.name,
+Shape(1 +: f.shape.toVector), f.dtype, 'N' +: f.layout) )
+batchIndex = 1
+ }
+
+ protected val mxNetHandler = MXNetHandler()
+
+ protected val mod = loadModule()
+
+ /**
+* This method will take input as IndexedSeq one dimensional arrays and
creates
+* NDArray needed for inference. The array will be reshaped based on the
input descriptors.
+*
+* @param input : A IndexedSequence of Java one-dimensional array, An
IndexedSequence is
+* is needed when the model has more than one input/output
+* @return IndexedSequence array of outputs.
+*/
+ override def predict(input: IndexedSeq[Array[Float]]):
IndexedSeq[Array[Float]] = {
+
+require(input.length == inputDescriptors.length, "number of inputs
provided: %d" +
+ " do not match number of inputs in inputDescriptors:
%d".format(input.length,
+
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167599886
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/PredictBase.scala
##
@@ -0,0 +1,200 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import ml.dmlc.mxnet.io.NDArrayIter
+import ml.dmlc.mxnet.{DataDesc, NDArray, Shape}
+import ml.dmlc.mxnet.module.Module
+
+import scala.collection.mutable.ListBuffer
+import org.slf4j.LoggerFactory
+
+/**
+ * Base Trait for MXNNet Predictor classes.
+ */
+trait PredictBase {
+
+ /**
+* This method will take input as IndexedSeq one dimensional arrays and
creates
+* NDArray needed for inference. The array will be reshaped based on the
input descriptors.
+* @param input: A IndexedSequence of Java one-dimensional array, An
IndexedSequence is
+* is needed when the model has more than one input/output
+* @return IndexedSequence array of outputs.
+*/
+ def predict(input: IndexedSeq[Array[Float]]): IndexedSeq[Array[Float]]
+
+ /**
+* Predict using NDArray as input. This method is useful when the input is
a batch of data
+* or when multiple operations on the input/output have to performed.
+* Note: User is responsible for managing allocation/deallocation of
NDArrays.
+* @param input: IndexedSequence NDArrays.
+* @return output of Predictions as NDArrays.
+*/
+ def predictWithNDArray(input: IndexedSeq[NDArray]): IndexedSeq[NDArray]
+
+}
+
+/**
+ * Implementation of predict routines.
+ *
+ * @param modelPathPrefix PathPrefix from where to load the model.
+ *Example: file://model-dir/resnet-152(containing
resnet-152-symbol.json,
+ *resnet-152-.params and optionally synset.txt).
+ *Supports model loading from various sources like
local disk,
+ *hdfs, https and s3. file://, hdfs://, https://,
s3://
+ * @param inputDescriptors Descriptors defining the input node names, shape,
+ * layout and Type parameters
+ * @param outputDescriptors Descriptors defining the output node names, shape,
+ * layout and Type parameters
+ */
+class Predictor(modelPathPrefix: String,
+ protected val inputDescriptors: IndexedSeq[DataDesc],
+ protected var outputDescriptors:
+ Option[IndexedSeq[DataDesc]] = None) extends PredictBase {
+
+ private val logger = LoggerFactory.getLogger(classOf[Predictor])
+
+ protected var batchIndex = inputDescriptors(0).layout.indexOf('N')
+ protected var batchSize = if (batchIndex != -1 )
inputDescriptors(0).shape(batchIndex) else 1
+
+ protected var iDescriptors = inputDescriptors
+
+ inputDescriptors.foreach((f: DataDesc) => require(f.layout.indexOf('N') ==
batchIndex,
+"batch size should be in the same index for all inputs"))
+
+
+ if (batchIndex != -1) {
+inputDescriptors.foreach((f: DataDesc) => require(f.shape(batchIndex) ==
batchSize,
+ "batch size should be same for all inputs"))
+ } else {
+// TODO: this is assuming that the input needs a batch
+iDescriptors = inputDescriptors.map((f : DataDesc) => new DataDesc(f.name,
+Shape(1 +: f.shape.toVector), f.dtype, 'N' +: f.layout) )
+batchIndex = 1
+ }
+
+ protected val mxNetHandler = MXNetHandler()
+
+ protected val mod = loadModule()
+
+ /**
+* This method will take input as IndexedSeq one dimensional arrays and
creates
+* NDArray needed for inference. The array will be reshaped based on the
input descriptors.
+*
+* @param input : A IndexedSequence of Java one-dimensional array, An
IndexedSequence is
+* is needed when the model has more than one input/output
+* @return IndexedSequence array of outputs.
+*/
+ override def predict(input: IndexedSeq[Array[Float]]):
IndexedSeq[Array[Float]] = {
+
+require(input.length == inputDescriptors.length, "number of inputs
provided: %d" +
+ " do not match number of inputs in inputDescriptors:
%d".format(input.length,
+
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167597820
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
+
+}
+
+object MXNetHandlerType extends Enumeration {
+
+ type MXNetHandlerType = Value
+ val SingleThreadHandler = Value("MXNetSingleThreadHandler")
+ val OneThreadPerModelHandler = Value("MXNetOneThreadPerModelHandler")
Review comment:
Given that one-thread-per-model is (to the best of my knowledge) currently
an unsafe operating mode, is there any point in supporting it just yet?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] calumleslie commented on a change in pull request #9678: [First cut] Scala Inference APIs
calumleslie commented on a change in pull request #9678: [First cut] Scala
Inference APIs
URL: https://github.com/apache/incubator-mxnet/pull/9678#discussion_r167601431
##
File path:
scala-package/infer/src/main/scala/ml/dmlc/mxnet/infer/MXNetHandler.scala
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package ml.dmlc.mxnet.infer
+
+import java.util.concurrent._
+
+trait MXNetHandler {
+
+ def execute[T](f: => T): T
+
+ val executor: ExecutorService
Review comment:
Arguably this should not be on the trait at all.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
