Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve the documentation #258

Merged
merged 63 commits into from Apr 4, 2019
Merged

Improve the documentation #258

merged 63 commits into from Apr 4, 2019

Conversation

pratikvn
Copy link
Member

@pratikvn pratikvn commented Mar 8, 2019 •
edited

This PR is concerned with the improvement of documentation in general for Ginkgo using Doxygen. This closes #76 .

Main changes:

  • Concept of modules to organize the documentation.
  • An explicit generated DoxygenLayout.xml file to modify different tabs automatically generated by doxygen.

Things to discuss:

  • The different colors for the top title bar. To be set in stylesheet.css
    @hartwiganzt has a list of suitable colors to choose from

  • How do we incorporate the examples and tutorials ? The wiki currently has an incomplete tutorial that could form the basis for this.

  • Remove fix-docs from deployment step before merging.

See how it looks here.

@pratikvn pratikvn added the reg:documentation This is related to documentation. label Mar 8, 2019
@pratikvn pratikvn added this to the Ginkgo 1.0.0 release milestone Mar 8, 2019
@pratikvn pratikvn self-assigned this Mar 8, 2019
@pratikvn pratikvn added the 1:ST:do-not-merge Please do not merge PR this yet. label Mar 8, 2019
@pratikvn
Copy link
Member Author

pratikvn commented Mar 8, 2019

@tcojean, I wanted to publish the documentation generated by this branch fix-docs on gh-pages, so that everyone can have a look. I added the fix-docs branch as well to the list for the deploy stage, but there seems to be some authentication error .

@tcojean
Copy link
Member

tcojean commented Mar 10, 2019

@pratikvn Yes, the variables used (for the bot login etc) are protected, so the branch needs to be protected for the deployment to work.

@pratikvn
Copy link
Member Author

I see. I dont really want to make the branch protected. Do you have any other suggestions ?

@tcojean
Copy link
Member

tcojean commented Mar 11, 2019 •
edited

I think protecting the branch is the only safe way. If we unprotect the variables then anyone could access their value through some CI job...

You can protect it, launch a CI pipeline, then unprotect it once it is completed.

@pratikvn
Copy link
Member Author

Okay. But I don't think I have permissions to make it protected either. I think that should be done through ginkgo-bot ?

thoasm
thoasm reviewed Mar 13, 2019
View reviewed changes
Copy link
Member

@thoasm thoasm left a comment

Choose a reason for hiding this comment

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

Currently, only gko::group is showing up when you look at the namespaces in doxygen. This is probably because it is commented with doxygen in cooperative_groups.cuh.

Can you also add comments to the other namespaces in order for them to show up?
I am not sure if it is enough to just comment in one file on a namespace in order for all other functions in other files to show up.

doc/headers/linop.hpp Outdated Show resolved Hide resolved
doc/headers/linop.hpp Outdated Show resolved Hide resolved
doc/headers/linop.hpp Outdated Show resolved Hide resolved
doc/headers/linop.hpp Outdated Show resolved Hide resolved
doc/headers/linop.hpp Outdated Show resolved Hide resolved
doc/pages/DOCUMENTATION.md Outdated Show resolved Hide resolved
doc/pages/DOCUMENTATION.md Outdated Show resolved Hide resolved
include/ginkgo/core/base/lin_op.hpp Show resolved Hide resolved
@tcojean
Copy link
Member

tcojean commented Mar 13, 2019

@pratikvn I made the branch protected and ran the documentation generation.

See: https://gitlab.com/ginkgo-project/ginkgo-public-ci/-/jobs/176738804
Doc: https://ginkgo-project.github.io/ginkgo/doc/fix-docs/

@tcojean
Copy link
Member

tcojean commented Mar 13, 2019

@pratikvn it's a nice improvement. I haven't explored everything but there is a bug with the Examples tab.

@pratikvn
Copy link
Member Author

Yes, I saw that. Thank you. Regarding the examples tab, that is one thing we have to discuss, I guess. Should we go for a proper tutorial or link to just our examples in the repo. Therefore, for now I left it unlinked. But i will link the wiki as default.

@tcojean
Copy link
Member

tcojean commented Mar 13, 2019 •
edited

@pratikvn On the issues you raised, I think the color #f8c369 looks good (I tried it by editing the page's CSS). Maybe a grey would integrate better although it would be very neutral and we do not have a grey color in the list.

For the examples tab, I believe the Wiki would be a good place to point to, but nonetheless we may need a full list of the examples (hopefully with documentation) which Ginkgo contains. Is there any way to generate such a thing? It would be great to have a list of examples with short description, eventually links to the related files for each example and there documentation for the class or whatever is implemented (such as the 3pt stencil). I think overall, we should have a page like that for examples and a separate one for tutorial which would point to the wiki entries.

A minor point, the header states develop, is there a way to change (or remove) that altogether?

@pratikvn
Copy link
Member Author

I have updated the color to #f8c369 now.

I have pointed the examples tab to the wiki for now, but as you suggest, we definitely should have a more comprehensive explanation in the documentation. I will see what I can add.

The header states the current branch and the version of code. It gets automatically updated from the appropriate cmake variables. So for the master branch, it should say something like master and 1.0.0. I think that is quite useful, so I think we should leave it there.

@tcojean
Copy link
Member

tcojean commented Mar 14, 2019

@pratikvn the documentation is now updated. Looks like the namespaces are properly fixed among other things.

As for the documentation showing develop, I meant that it should probably print fix-docs currently because that is the name of your branch. I will see what can be done for that.

@pratikvn pratikvn force-pushed the fix-docs branch 2 times, most recently from 30f474b to 1621678 Compare March 14, 2019 11:58
@tcojean tcojean mentioned this pull request Mar 14, 2019
Merged
tcojean added a commit that referenced this pull request Mar 15, 2019
@tcojean
Merge pull request add codecov
de045ff 
This is a small PR with the purpose of adding a new tool, [codecov](https://codecov.io/gh/ginkgo-project/ginkgo/branch/add_codecov).

This provides the ability to add a coverage banner in our Readme. I also add some other convenience banners at the same time. The codecov tool itself provides a different view of code coverage which is fairly intuitive. See the link above for an example.
Here is a sample from the branch:  ![Coverage](https://codecov.io/gh/ginkgo-project/ginkgo/branch/add_codecov/graph/badge.svg)

Finally, it is unrelated but the containers are updated at the same time to also add the `graphviz` tool which is required for PR #258.

Related PR: #264
@thoasm
Copy link
Member

thoasm commented Mar 15, 2019

Currently, in the normal documentation, we also list the kernel namespace with all members, so including all omp, cuda and reference kernels. Is there a way to only generate the documentation for files inside include/ginkgo?

@tcojean
Copy link
Member

tcojean commented Mar 15, 2019 •
edited

I would add, and for examples and benchmarks for which documentation would be nice. The things we do not need to generate documentation for (for the public) are in core, cuda, omp and reference, I think.

pratikvn and others added 22 commits April 4, 2019 13:39
@pratikvn
Add a modules dot graph
f47a6b1
@pratikvn
Add a forgotten file.
3f86fb8
@pratikvn
Correct namespaces and remove duplicates, remove duplicate dev_doc, r…
8045ac7 
…emove whitespaces
@pratikvn
Generate doc from source dir, not from doc folder. Add tabs for insta…
99d0213 
…ll, test and bench, Add missing benchmarking.md file
@pratikvn
Add an example section in README, remove old mentions of example-* in…
545b6fc 
… scripts.
@pratikvn
Remove the modules graph.
25085c4
@pratikvn
Some more internal tags to be removed.
89121ed
@pratikvn
Add deal.II license. Fix some typos and style issues.
f0a50f9
@pratikvn
Streamline some grouping in linop and log, Maintain uniformity with n…
0d7578c 
…ew lines
@pratikvn
Add complete sentences to the example descriptions, thanks to @thoasm
ab6f2e1
@pratikvn
Add some documentation for mtx_io class.
6a50e38
@pratikvn
Add testing note to README.md
0318768
@pratikvn
Move initialize to mat formats, add some documentation for mtx_io, re…
664ab4d 
…move commented stuff in filter
@pratikvn
Move the separator creation to a variable.
0fef6dc
@tcojean @pratikvn
Fix format of some markdown files. Use some md files as doc pages.
78c7ac6
@pratikvn
Add the concatenation of the mainpage for doc. rename some functions …
e1af142 
…in doc cmake helpers
@tcojean @pratikvn
Add documentation generation for the PAPI logger class. Fixes some DE…
97317b1 
…V warnings.
@pratikvn
Use defined variables instead of CMake variables in helpers cmake, doc.
3f3fc57
@pratikvn
Remove our license from the deal.II example to comply with the LGPL l…
34de257 
…icense. Addexception to the add_license script
@pratikvn
Fix the navindex error in the xml file.
006ded1
@pratikvn
Fix pdf generation problems with setting INLINE_SOURCES=YES to resolv…
bc580ea 
…e hyperlinks
@pratikvn
Fix the about license structure for deal.ii.
9ce8e9a
@pratikvn pratikvn added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:do-not-merge Please do not merge PR this yet. 1:ST:ready-for-review This PR is ready for review labels Apr 4, 2019
@pratikvn pratikvn merged commit 414bee7 into develop Apr 4, 2019
@pratikvn pratikvn deleted the fix-docs branch April 4, 2019 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcojean tcojean tcojean approved these changes

@thoasm thoasm thoasm approved these changes

@gflegar gflegar Awaiting requested review from gflegar

@hartwiganzt hartwiganzt Awaiting requested review from hartwiganzt

Assignees

@pratikvn pratikvn

Labels
1:ST:ready-to-merge This PR is ready to merge. reg:documentation This is related to documentation.
Projects
None yet
Milestone
Ginkgo 1.0.0 release
Development

Successfully merging this pull request may close these issues.

Improvements to Doxygen
4 participants
@pratikvn @tcojean @thoasm @hartwiganzt