[MXNET-319] Javadoc fix #11239

lanking520 · 2018-06-12T01:05:37Z

Description

This is the second proposed solution to this: #11123
Add Java doc support.
@nswamy @yzhliu @andrewfayres
Be aware, I have imported a third party library to generate the Scala doc. Please check the pom file carefully.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

nswamy · 2018-06-12T18:24:50Z

scala-package/native/osx-x86_64-cpu/pom.xml

@@ -67,7 +67,7 @@
            <linkerMiddleOption>-Wl,-x</linkerMiddleOption>
            <linkerMiddleOption>${lddeps}</linkerMiddleOption>
            <linkerMiddleOption>-force_load ../../../lib/libmxnet.a</linkerMiddleOption>
-            <linkerMiddleOption>-force_load ../../../3rdparty/tvm/nnvm/lib/libnnvm.a</linkerMiddleOption>
+            <linkerMiddleOption>-force_load ../../../3rdparty/nnvm/lib/libnnvm.a</linkerMiddleOption>


Why is this required? ${lddeps} should already have these ? see how it is setup
https://github.com/apache/incubator-mxnet/blob/master/Makefile#L386
https://github.com/apache/incubator-mxnet/blob/master/Makefile#L581

I am not sure, it just live there for a long time. And do you know who change that line of code. I pulled from master and see that addition

nswamy · 2018-06-14T21:54:46Z

scala-package/core/src/main/scala/org/apache/mxnet/Symbol.scala

@@ -823,7 +823,7 @@ class Symbol private(private[mxnet] val handle: SymbolHandle) extends WarnIfNotD
 }

 @AddSymbolFunctions(false)
-object Symbol {
+object Symbol extends SymbolBase {


please add comments to all class/object/method definitions.

nswamy · 2018-06-14T21:55:20Z

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala

@@ -37,6 +37,8 @@ private[mxnet] object APIDocGenerator{
    val FILE_PATH = args(0)
    absClassGen(FILE_PATH, true)
    absClassGen(FILE_PATH, false)
+    oldAPIClassGen(FILE_PATH, true)


oldAPIClassGen->NonTypeSafeClassGen

nswamy · 2018-06-14T21:55:39Z

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala

@@ -61,8 +63,35 @@ private[mxnet] object APIDocGenerator{
    pw.close()
  }

+  def oldAPIClassGen(FILE_PATH : String, isSymbol : Boolean) : Unit = {


nonTypeSafeAPIClassGen

nswamy · 2018-06-14T21:56:51Z

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala

  // Generate ScalaDoc type
-  def generateAPIDocFromBackend(func : absClassFunction) : String = {
+  def generateAPIDocFromBackend(func : absClassFunction, withParam : Boolean = true) : String = {


could you please make sure that the documentation matches what we have today, i think we follow Java doc style and not Scala doc style..make sure its consistent as well. I can't tell from the text

I followed this guideline: https://docs.scala-lang.org/style/scaladoc.html

nswamy · 2018-06-14T22:30:07Z

scala-package/core/pom.xml

+        <artifactId>scalastyle-maven-plugin</artifactId>
+      </plugin>
+      <plugin>
+        <groupId>net.alchim31.maven</groupId>


why are we using the plugin from this group? we use it from this group org.scala-tools. Please check pom file on scala-package.

Currently I cannot find a better approach to generate a doc jar rather than this plugin. As we discovered before, the Javadoc plugin that Scala Package originally have did not compile the Scala code and generate jars.

lanking520 · 2018-06-15T23:26:07Z

Currently, I created a file called FILEHASH lives in the core used to check if there is a change on generated base files. Add a require as well to alert developers. Please check the Hash file and leave your thinkings. I used MD5 in here

lanking520 · 2018-06-18T16:26:32Z

Currently, I cannot make it pass the GPU test even if the MD5 is guaranteed unchanged. I cannot reproduce the problem even if I open a GPU instance and build all over again. In this case, I decided to remove GPU test from the the MD5 checking. It seemed the code generation result are someway different between CIs. Need to figure out in the future

szha · 2018-06-18T17:33:52Z

scala-package/core/src/main/scala/org/apache/mxnet/FILEHASH

@@ -0,0 +1,4 @@
+309G_O39xojNEt3DXVWNDQ


Is this meant to be checked in? what files does this hash represent? how are they going to be updated?

Good question. The hashfile contains the signature files that on Scala package not uploaded here. It follows the following format:

abstract class SymbolAPIBase { /** * Comments */ def Activation(...Args...) : Symbol }

I used md5 hash in here:

def MD5Generator(input : String) : String = { val md = MessageDigest.getInstance("MD5") md.update(input.getBytes("UTF-8")) val digest = md.digest() org.apache.commons.codec.binary.Base64.encodeBase64URLSafeString(digest) }

It basically take the Generated string into hash and store in this file. It should be the same across the different platform. The reason to do is is to avoid operator changes in Symbol and NDArray. One example, if one contributor changed the operators in NDArray and Symbol, he/she should comment the require line explained before in Scala package and update this file and also tag Scala Contributors to let them know there is a change in operator so we can determine if this will influence the user experience on Scala.

nswamy

Thanks @lanking520. Great effort, one last request to change.

nswamy · 2018-06-29T11:06:18Z

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala

+  def nonTypeSafeClassGen(FILE_PATH : String, isSymbol : Boolean) : String = {
+    // scalastyle:off
+    val absClassFunctions = getSymbolNDArrayMethods(isSymbol)
+    val absFuncs = absClassFunctions.filterNot(_.name.startsWith("_")).map(absClassFunction => {


Can you move this filter to the getSymbolNDArrayMethods, so we have one place to change in when start adding support for _sparse_ etc.,

I think we should also do this for the NDArrayMacro/SymbolMacro in a separate PR

@lanking520 please change the filter location, so i can merge. I meant in APIDocGenerator

Hi @nswamy I have done that, please double check in the code

Attempt to create universal solution to all platforms: https://stackoverflow.com/questions/3077196/messagedigest-hashes-differently-on-different-machines

* Create Interface for Symbol and NDArray APIs, enable JavaDoc jar building for Scala Package.

lanking520 requested a review from yzhliu as a code owner June 12, 2018 01:05

lanking520 mentioned this pull request Jun 12, 2018

[MXNET-319] Javadoc fix #11143

Closed

5 tasks

nswamy reviewed Jun 12, 2018

View reviewed changes

lanking520 requested a review from szha as a code owner June 13, 2018 18:14

lanking520 force-pushed the javadoc-fix3 branch 2 times, most recently from e9ab9d7 to 4d1954f Compare June 14, 2018 18:04

nswamy suggested changes Jun 14, 2018

View reviewed changes

lanking520 force-pushed the javadoc-fix3 branch from 23235c8 to 0992da7 Compare June 14, 2018 22:59

lanking520 force-pushed the javadoc-fix3 branch from ab3238c to e556dca Compare June 18, 2018 06:45

szha reviewed Jun 18, 2018

View reviewed changes

lanking520 force-pushed the javadoc-fix3 branch 4 times, most recently from 167dacf to 980109b Compare June 23, 2018 22:43

lanking520 force-pushed the javadoc-fix3 branch 2 times, most recently from 9d4f790 to 7d74310 Compare June 28, 2018 21:26

nswamy suggested changes Jun 29, 2018

View reviewed changes

lanking520 force-pushed the javadoc-fix3 branch from 3fe0565 to 0d00903 Compare June 29, 2018 17:15

lanking520 added 10 commits July 1, 2018 12:47

add init commit

18bda36

revert the native config

7dfa680

add pom

d1a9083

Add definitions and name changes

74d4969

Add hashcode for all file that is being generated

47504ba

Add operational support

27d6d30

Add UTF-8 as default bytes coder

4a7f20c

Add SafeString solution

749adf5

Attempt to create universal solution to all platforms: https://stackoverflow.com/questions/3077196/messagedigest-hashes-differently-on-different-machines

Skip GPU test

dc7d373

Add CI changes

cf56005

lanking520 added 9 commits July 1, 2018 12:47

change hash

0fc967a

make it more instructive

d3aaa23

revert hashcheck and add more info in the error message

36ee375

update test case

992ed10

fix small issues

b3ac603

add pom changes for inference and spark

bf14773

fixing existing issues and add <br>

dc04942

change the filter location

2f84b76

amend uncommitted changes

f6aa970

lanking520 force-pushed the javadoc-fix3 branch from d1666cf to f6aa970 Compare July 1, 2018 19:47

nswamy merged commit 815eb60 into apache:master Jul 3, 2018

lanking520 deleted the javadoc-fix3 branch July 3, 2018 22:36

XinYao1994 pushed a commit to XinYao1994/incubator-mxnet that referenced this pull request Aug 29, 2018

[MXNET-319] Javadoc fix (apache#11239)

b20682f

* Create Interface for Symbol and NDArray APIs, enable JavaDoc jar building for Scala Package.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-319] Javadoc fix #11239

[MXNET-319] Javadoc fix #11239

lanking520 commented Jun 12, 2018 •

edited

Loading

nswamy Jun 12, 2018

lanking520 Jun 12, 2018

nswamy Jun 14, 2018

nswamy Jun 14, 2018

nswamy Jun 14, 2018

nswamy Jun 14, 2018

lanking520 Jun 14, 2018

nswamy Jun 14, 2018

lanking520 Jun 14, 2018

lanking520 commented Jun 15, 2018

lanking520 commented Jun 18, 2018

szha Jun 18, 2018

lanking520 Jun 18, 2018 •

edited

Loading

nswamy left a comment

nswamy Jun 29, 2018

nswamy Jun 29, 2018 •

edited

Loading

nswamy Jul 1, 2018 •

edited

Loading

lanking520 Jul 1, 2018

[MXNET-319] Javadoc fix #11239

[MXNET-319] Javadoc fix #11239

Conversation

lanking520 commented Jun 12, 2018 • edited Loading

Description

Checklist

Essentials

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanking520 commented Jun 15, 2018

lanking520 commented Jun 18, 2018

Choose a reason for hiding this comment

lanking520 Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

nswamy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nswamy Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

nswamy Jul 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanking520 commented Jun 12, 2018 •

edited

Loading

lanking520 Jun 18, 2018 •

edited

Loading

nswamy Jun 29, 2018 •

edited

Loading

nswamy Jul 1, 2018 •

edited

Loading