Documentation Refactor

[hogepodge]

RFC: https://github.com/apache/tvm-rfcs/blob/main/rfcs/0027-formalize-documentation-organization.md
Tracking Issue: #8987

Stage 1 of the documentation refactor reorganizes the docs structure,
moving files (without content changes) and adding new scaffolding to
generate the proper document tree.

It does not address naming, style, content, links, or other existing
content in documents that were moved. State 2 will address fixing these
issues with existing content.

Major changes include but are not limited to:

• Dividing the existing tutorials into two sections:
  • Tutorials
  • How Tos
• Moving all of the existing tutorials out of the /tutorial
  directory and into the more general /gallery directory.
• Breaking up how-tos into individual sections for more
  flexibility and more consistent rendering.
• Moving content into new classifications:
  • /docs/arch for architecture guides
  • /docs/reference for API guides and other reference material
  • /docs/topic for topic specific guides such as microTVM and VTA
  • Restructuring /docs/dev
• Adding a table of contents to the doc index
• Adding instructions on how to install using third-party tlcpack

Stage 2 of the documentation refactor fixes naming and links
in the documentation to be consistent with the overall structure.

Major changes include:

• an update to how to contribute to docs.
• several updated index pages with title changes to match
  the organization style and bring consistency to the sections
• expanded descriptions of some page collections
• fixed links

Stage 3 of the documentation refactor adjusts CI for the new structure.
The CI build script takes into account the new gallery format. It
also prevents deleting existing documents, and takes advantage of the
_staging and _build directories to clean out previous builds

[hogepodge]

Addressed missing links and references in documentation.

[tqchen]

Thanks Chris, the overall structure looks good and thanks for the effort in preserving the contribution history.
Looks like we still need to update the CI script in order to get the things to unblock the ci

See related lines here

• https://github.com/apache/tvm/blob/main/tests/scripts/task_python_docs.sh#L42
• https://github.com/apache/tvm/blob/main/tests/scripts/task_sphinx_precheck.sh#L30

Likely we need to update these to the new folder locations, and in the case of ci_logs, update them to match the new location of auto_scheduler(should be gallery/how_to/tune_with_autoscheduler?). Please let me know if you need additonal help.

[tqchen]

BTW, I wanted to acknowledge and thank you for all the effort you have put into the documentation. Having strong user and developer documentation is critical to TVM’s mission to provide unity to the ML Acceleration ecosystem,and the entire community stands to benefit from better documentation.

The overall direction of documentation changes looks great and layed out clearly in the RFC. Sorry for the final requests but these are the final steps toward closing the loop of our documentation refactor effort. Let's work together to land this and please let us know what we can do to help.

[tmoreau89]

Thank you @hogepodge for this colossal PR and equally colossal effort! These changes will have a hugely positive impact on improving TVM's approachability for the community at large. Thanks for moving the needle in the right direction

[denise-k]

Huge, huge thanks to @hogepodge for spending so much time and energy to make our documentation shine! ✨

[hogepodge]

Thanks Chris, the overall structure looks good and thanks for the effort in preserving the contribution history. Looks like we still need to update the CI script in order to get the things to unblock the ci

See related lines here

• https://github.com/apache/tvm/blob/main/tests/scripts/task_python_docs.sh#L42
• https://github.com/apache/tvm/blob/main/tests/scripts/task_sphinx_precheck.sh#L30

Likely we need to update these to the new folder locations, and in the case of ci_logs, update them to match the new location of auto_scheduler(should be gallery/how_to/tune_with_autoscheduler?). Please let me know if you need additonal help.

Thanks! I've updated the scripts. A little while ago @Lunderberg changed the build structure to use both a _staging and a _build directory. I've updated the scripts to see the new locations and to only remove those directories (as the main tree is not being written in to any more).

[tqchen]

The remaining error seems was due to tvm not being available in some of the how to example executions. It might be helpful to run the following command to quickly check a specific tutorial to debug

TVM_TUTORIAL_EXEC_PATTERN=/path/to/specific_file\.py make html

It could be related to the staging setup, the way we setup env variables (cc @Lunderberg to see if you have any ideas).

[hogepodge]

Thanks for the suggestion on that. Whether I'm building the entire document set, or just the individual how_tos that are failing in CI, I'm unable to reproduce the failures locally.

[hogepodge]

I just pushed a new change to see if we can reproduce the same failures in CI.

[Lunderberg]

Hmm. It looks like it's having trouble finding the libtvm.so file, but it finds it correctly during the initial import. From a quick 10-minute code dive, it looks like sphinx_gallery calls memory_profiler.memory_usage. This starts a subprocess, which doesn't inherit the sys.path updates made in our conf.py.

Can you try adding os.environ['PYTHONPATH'] = os.pathsep.join(sys.path) to the conf.py, just after the modifications to sys.path? The environment variable should be passed to subprocesses, where the value of sys.path is not.

[manupak]

Is this related : https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/PR-9168/8/pipeline ? (cc: @Mousius )

[hogepodge]

@manupa-arm looks like the same issue

[hogepodge]

@Lunderberg trying that out, it makes sense. In my local dev environment I actually install TVM to a user python package, which may explain why I'm not seeing the local failures.

[hogepodge]

Fixed linting error in conf.py

[hogepodge]

@Lunderberg I made the suggested changed, but that didn't improve it. After digging into the Sphinx Gallery code a bit more, I don't think the memory_usage code is being called. It's disabled by default. Seems like this is going to need some deeper digging. I don't see anything right away that would explain these failures.

[manupak]

@hogepodge , I've just experienced another failure. I remember we had an issue which got resolved by @mbs-octoml PR, IIRC. Do we need to re-open that or create a new one as I dont immediately see this issue is relevant to the failure.

[mbs-octoml]

That was my bad and the fix #9227 is merged. Why it showed up now and not during CI for #8788 is a mystery.

…

On Wed, Oct 6, 2021 at 11:04 PM Manupa Karunaratne ***@***.***> wrote: @hogepodge <https://github.com/hogepodge> , I've just experienced another failure. I remember we had an issue which got resolved by @mbs-octoml <https://github.com/mbs-octoml> PR, IIRC. Do we need to re-open that or create a new one as I dont immediately see this issue is relevant to the failure. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9203 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUYORLF5IOU4ZJTU3UF5KYTUFUZ5PANCNFSM5FMKRM2A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[hogepodge]

I'm not convinced this is going to fix the underlying problem, but we'll see where we are in a few hours.

[mbs-octoml]

What is the underlying problem?

…

On Fri, Oct 8, 2021 at 8:56 AM Chris Hoge ***@***.***> wrote: I'm not convinced this is going to fix the underlying problem, but we'll see where we are in a few hours. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9203 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUYORLEXMNYFI4QRJGG7XI3UF4IEPANCNFSM5FMKRM2A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[hogepodge]

The number of failures for this PR was more widely spread across a number of tutorials, and manifested as not being able import python modules and associated libraries. It seems like sphinx-gallery is brittle and has a global python state that is easily corrupted, and I'm wondering if my reordering of the how-tos is causing failures based on those changes. Your patch fixed an instance of that, but my worry is that there are more problems like that lurking. Hopefully I'm wrong about that though.

[areusch]

marking request changes for the config.cmake change--i think i see your issue.

[tqchen]

OK, I find sometime to dig a bit deeper into the errors. The problem comes from the way we get tvm_path https://github.com/apache/tvm/blob/main/docs/conf.py#L52 which is relative instead of absolute path.

Sphinx gallery runs the tutorial file from cwd of the file(e.g. it will run on gallery/how_to/compile_models/) where the environment variable of tvm_path set to ../../python produces a mismatch.

A possible way to fix is to add tvm_path = os.path.abs_path(tvm_path) to https://github.com/apache/tvm/blob/main/docs/conf.py#L54(and I confirm it works)

But given @Lunderberg you wrote this part and left a comment about relative path, would be great to confirm

[Lunderberg]

The issue was that ExplicitOrder requires relative paths, and tvm_path is currently used to generate the arguments for ExplcitOrder. (I had to verify to remind myself why. The implementation of ExplicitOrder expects the string representation of the paths to be an exact match for the path being used. It gets passed the path relative to the source directory, and so that's what we need to give it.) If I remember correctly, I think the failure mode was generating URLs that included the full path to the file, including /home/Lunderberg.

The path being added to sys.path doesn't need to be a relative path, so we could change tvm_path to tvm_path.resolve() on those two lines.

[hogepodge]

@Lunderberg I just came to the same conclusion, and addressed it by making a tvm_path_rel variable to use in the Sphinx Gallery ExplicitOrder.

The requests are fulfilled

[tqchen]

Thanks @hogepodge @Lunderberg @areusch . All the comments are resolved.

I am merging this in given that this is a major refactor could conflict/block future changes and would benefit from prioritization. Let us send followup PRs for future tweaks.