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

[MXNET-1164] Generate the document for cpp-package using Doxygen #12977

Merged
merged 5 commits into from
Dec 20, 2018
Merged

[MXNET-1164] Generate the document for cpp-package using Doxygen #12977

merged 5 commits into from
Dec 20, 2018

Conversation

leleamol
Copy link
Contributor

Description

The PR includes changes to Doxyfile for generating documents for cpp-package. Also modified the index.md file in api/c++ directory to include brief build instructions and links to the correct classes in mxnet::cpp namespace

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • [y] The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • [y] 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

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)
    Verified that the doxygen generates the docs for cpp-package.

Comments

@leleamol leleamol requested review from nswamy and szha as code owners October 25, 2018 23:49
@ankkhedia
Copy link
Contributor

@leleamol Thanks for your contribution!

@mxnet-label-bot [pr-awaiting-review, C++]

@marcoabreu marcoabreu added C++ Related to C++ pr-awaiting-review PR is waiting for code review labels Oct 29, 2018
@leleamol leleamol mentioned this pull request Oct 31, 2018
Open
@ankkhedia
Copy link
Contributor

@nswamy Could you please take a look into this PR?

@nswamy
Copy link
Member

nswamy commented Nov 8, 2018

could you paste the generated doxygen files?

@kalyc
Copy link
Contributor

kalyc commented Nov 13, 2018

ping @leleamol ! could you please provide the generated doxygen files?

@leleamol
Copy link
Contributor Author

@nswamy There will be a doxygen file for each hpp and h file in the cpp-package folder.

Should I paste all those files here?

@stu1130
Copy link
Contributor

stu1130 commented Nov 21, 2018

@nswamy ping

@leleamol
Copy link
Contributor Author

@aaronmarkham

aaronmarkham
aaronmarkham reviewed Nov 28, 2018
View reviewed changes
Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

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

The preview that should exist for this PR isn't there.
Do you have a dev web server that we can preview?

docs/Doxyfile
@@ -770,7 +770,7 @@ WARN_LOGFILE =
# spaces.
# Note: If this tag is empty the current directory is searched.

INPUT = include src/common
INPUT = include src/common cpp-package/include/mxnet-cpp
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this combining the C and C++ API together in one Doxygen output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aaronmarkham Yes for now it is combining C and C++ API together until we find out the better way to separate them. C++ API doc will be available under "cpp" namespace and won't collude with the existing doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont have the dev web server that is publicly accessible for preview.

@aaronmarkham
Copy link
Contributor

Restarted the centos CI job... failed due to running out of space.

@Roshrini
Copy link
Member

@leleamol Can you retrigger the CI for this PR?
@mxnet-label-bot Update [pr-awaiting-testing]

@marcoabreu marcoabreu added pr-awaiting-testing PR is reviewed and waiting CI build and test and removed C++ Related to C++ pr-awaiting-review PR is waiting for code review labels Dec 19, 2018
@aaronmarkham
Copy link
Contributor

@leleamol Please add 3rdparty to the EXCLUDE directive in Doxyfile. That should help retrigger CI as well as provide the additional benefit of limiting the doxygen run to relevant files.

I don't know why it runs on 3rdparty - you'd think that the INPUT directive limits things, but if you look at the logs you see:

Generating file sources...
Generating code for file 3rdparty/dlpack/include/dlpack/dlpack.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/any.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/array_view.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/base.h...
Generating code for file 3rdparty/mshadow/mshadow/base.h...
Generating code for file include/mxnet/base.h...
Generating code for file 3rdparty/tvm/nnvm/include/nnvm/base.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/blockingconcurrentqueue.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/common.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/concurrency.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/concurrentqueue.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/config.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/data.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/endian.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/input_split_shuffle.h...
Generating code for file 3rdparty/dmlc-core/include/dmlc/io.h...
...

I tried excluding the 3rdparty folder and that does seem to work... it does a pass through the folder making an exclusion list.

WDYT?

@leleamol
Copy link
Contributor Author

Hi @aaronmarkham I have updated the Doxyfile and excluded the 3rdparty folder.
I verified that cpp-package doc gets correctly generated and 3rdparty folders are also getting excluded.

@aaronmarkham aaronmarkham merged commit 80ec46c into apache:master Dec 20, 2018
@leleamol
Copy link
Contributor Author

@mxnet-label-bot remove [ pr-awaiting-testing ]

@marcoabreu marcoabreu removed the pr-awaiting-testing PR is reviewed and waiting CI build and test label Dec 21, 2018
rondogency pushed a commit to rondogency/incubator-mxnet that referenced this pull request Jan 9, 2019
@leleamol
[MXNET-1164] Generate the document for cpp-package using Doxygen (apa…
2b1698e 
…che#12977)

* Adding cpp-package directory to the Doxyfile. Updating the index.md file in c++ api directory.

* Updating the link to classes in C++ API to point to correct html file.

* Updated the links to use relative paths.

* Removed the extra slash character in the url

* Excluded the 3rdparty folder as per the review comment.
@leleamol leleamol deleted the cpp-package-doc branch January 23, 2019 21:08
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
@leleamol
[MXNET-1164] Generate the document for cpp-package using Doxygen (apa…
e28396c 
…che#12977)

* Adding cpp-package directory to the Doxyfile. Updating the index.md file in c++ api directory.

* Updating the link to classes in C++ API to point to correct html file.

* Updated the links to use relative paths.

* Removed the extra slash character in the url

* Excluded the 3rdparty folder as per the review comment.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aaronmarkham aaronmarkham aaronmarkham left review comments

@nswamy nswamy Awaiting requested review from nswamy nswamy is a code owner

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
Doc Website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@leleamol @ankkhedia @nswamy @kalyc @stu1130 @aaronmarkham @Roshrini @marcoabreu