-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-1000] get Ndarray real value and form it from a NDArray #12690
Conversation
@mxnet-label-bot [pr-awaiting-review] |
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
I am not comfortable adding this API, an analogy to NDArray is BufferedImage, you don't print all the pixel values of a BufferedImage in toString. I understand it helps with debugging, however i would expect the developer/user to use a debugger or override toString then. |
@nswamy two things I would like to mention:
|
IMO, It still doesn't warrant to add this , whether you call it visualize or override toString. Another thing its confusing user experience that you are putting arbitrary THRESHOLD values?. what makes these values to be correct, why not more or less.
|
@nswamy since this is just a helper tool for users to debug and see what content inside. I think it's fair to have this tool. This Threshold value is just a limitation on numbers of elements to be shown in the array. The reason I set it as 1000 because 10000 sometimes would crash the JVM because it used too much memory and it's making no sense for a user to visualize that much. If the user seriously wants something to show up, he/she should at least try to slice and get it visualized. We can get this threshold configurable and ask the user to pass in. I am also curious how numpy set up this number in Python. If I can find a number in there I will let you know. |
Link to the discussion we discussed before: #12536 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this API(visualize/toString in object) fit to be in the NDArray class, it might be suitable in a Util class.
Any reason why we should not name the function |
quoting from Oracle's toString https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#toString()
It has to be concise: It might be helpful to print the metadata of the NDArray(Shape, DType, ...). |
I agree. Can we modify the current impl so that it can reach the toString bar? |
78b872b
to
ebc14c4
Compare
This is a demo for toString method: val arrBuf = ArrayBuffer[Array[Float]]()
for (i <- 0 until 20) arrBuf += Array(1.0f, 2.0f, 3.0f, 4.0f)
val arr = Array(
Array(
arrBuf.toArray
),
Array(
arrBuf.toArray
)
)
val nd = NDArray.toNDArray(arr)
println(Visualize.toString(nd))
println(nd) Here is the output
Here is what Gluon looks like:
|
@nswamy @yzhliu @gigasquid WDYT? |
@nswamy Any thoughts? If it is for pretty-printing ndarray, I think it is suitable to put into toString. As for now we do not have a good toString for NDArray. |
f59e098
to
5a74776
Compare
As offline discussion with @nswamy, the visualize method will go to a Util class. The reason behind it does not quite explain the idea behind NDArray. We can take it back to |
would you mind describe at what extent, it describes the idea behind NDArray? I think such a small feature can be easily made correctly at the very first time. |
@yzhliu here is the points: We should not place it at the very core place if we are not totally confident about its functionality. We should place it elsewhere until we are confident enough user likes it and it is not buggy. |
My point is, if we think the method is buggy, then we should not place it anywhere - why create a new Class which for sure will be deprecated later? So, do we agree, NDArray needs an implement of |
So far what need to be improved:
Anything else? |
@yzhliu my concerns are:
Lastly look at what other frameworks are doing in their Tensor's toString() |
I think Tensor is an ideal translation to MXNet Symbol since Tensorflow itself is a symbolic language. It did not contain real-time data when you compute at its stage so showing the detailed information seemed impossible. I agree with you on showing the entire image buffered data is not necessary unless the user really want to do so. |
2a9f2c1
to
3e96e35
Compare
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/test/scala/org/apache/mxnet/NDArraySuite.scala
Outdated
Show resolved
Hide resolved
af53455
to
9f2ea34
Compare
scala-package/core/src/main/scala/org/apache/mxnet/MX_PRIMITIVES.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/test/scala/org/apache/mxnet/NDArraySuite.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
scala-package/core/src/main/scala/org/apache/mxnet/NDArray.scala
Outdated
Show resolved
Hide resolved
9f2ea34
to
a6a3b4f
Compare
scala-package/core/src/test/scala/org/apache/mxnet/NDArraySuite.scala
Outdated
Show resolved
Hide resolved
@zachgk Please review again |
…e#12690) * add visualize * adding Any type input to form NDArray * fix bug and add tests * add a toString method * add Visualize Util and migrate visualize structure to there * update with tests * refactor code * fix the minor issue * add multiple types support * add changes on names and tests * make code elegant and improve readability
…e#12690) * add visualize * adding Any type input to form NDArray * fix bug and add tests * add a toString method * add Visualize Util and migrate visualize structure to there * update with tests * refactor code * fix the minor issue * add multiple types support * add changes on names and tests * make code elegant and improve readability
…e#12690) * add visualize * adding Any type input to form NDArray * fix bug and add tests * add a toString method * add Visualize Util and migrate visualize structure to there * update with tests * refactor code * fix the minor issue * add multiple types support * add changes on names and tests * make code elegant and improve readability
…e#12690) * add visualize * adding Any type input to form NDArray * fix bug and add tests * add a toString method * add Visualize Util and migrate visualize structure to there * update with tests * refactor code * fix the minor issue * add multiple types support * add changes on names and tests * make code elegant and improve readability
Description
This is an implementation of visualizing the internal structure of NDArray as well as forming an NDArray from a multi-dimensional array.
visualize
: Shown the structure of the current NDArray, it is also able to visualize some large NDArraytoNDArray
: Convert a Array[Array[....Array[MXNET_PRIMITIVE]...]] matrix into NDArray. Flatten and fill in a long array@nswamy @andrewfayres @yzhliu @gigasquid @ustcfd
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.