Skip to content

[Tutorial] Fix formatting, grammar, dead link#9281

Merged
masahi merged 4 commits intoapache:mainfrom
mkroening:tutorial-fix
Oct 23, 2021
Merged

[Tutorial] Fix formatting, grammar, dead link#9281
masahi merged 4 commits intoapache:mainfrom
mkroening:tutorial-fix

Conversation

@mkroening
Copy link
Contributor

This fixes a few issues (formatting, dead links, grammar), I encountered when reading the tutorial.

@hogepodge, could you review this?

@mkroening mkroening requested a review from a team as a code owner October 14, 2021 08:50
@hogepodge
Copy link
Contributor

Thank you for catching these. I would like to request a change in how the links are handled. Instead of direct links, can we please use Sphinx reference links? This will help prevent breaks in the future.

For example, this file has a reference at the top of it:

We can write the link to that as:

:ref:`Auto-tuning a convolution network for x86 CPU <tune_relay_x86>`.

This will make the link more durable and less likely to break if the files are ever moved again. There is documentation for Sphinx that describes this feature more fully. This would be a great start on that. Thank you for sending up this patch, and catching links that I missed in the refactor.

@hogepodge
Copy link
Contributor

One more comment, if the target link doesn't have a reference tag as shown above, it should be added. Any name that doesn't collide with another tag should work. If you need me to build the docs and check once the patch is up, I'd be happy to help out with that.

@jroesch
Copy link
Member

jroesch commented Oct 14, 2021

Agree that it would be best to make the change @hogepodge suggested to avoid future breakage. Thanks for the fix!

@tqchen tqchen added the status: need update need update based on feedbacks label Oct 14, 2021
@mkroening
Copy link
Contributor Author

I pushed three more commits, the last of which adopts sphinx references.

@hogepodge, it would be great if you could build and check if it works.

@mkroening
Copy link
Contributor Author

I also noticed one code block1 that looks like having the comments not be outside the block might not be intentional.

Footnotes

  1. https://tvm.apache.org/docs/tutorial/tensor_expr_get_started.html#targeting-vector-addition-for-gpus-optional

@mkroening
Copy link
Contributor Author

I added another commit, fixing a link in README.md.

@hogepodge
Copy link
Contributor

hogepodge commented Oct 19, 2021

@mkroening I did a pass through the code and fixed a bunch of other links. Rather than send competing patching, can you check the differences in this patch and update to catch the remainder of direct links?

https://github.com/hogepodge/tvm/tree/fix-links

@hogepodge
Copy link
Contributor

Probably easier to compare from this PR: #9314

@mkroening mkroening force-pushed the tutorial-fix branch 2 times, most recently from 492268a to d26ed50 Compare October 19, 2021 07:04
@mkroening mkroening changed the title [Tutorial] Fix dead links, formatting [Tutorial] Fix formatting, grammar, dead link Oct 19, 2021
@mkroening
Copy link
Contributor Author

@hogepodge, I subtracted #9314 from this PR.

@mkroening
Copy link
Contributor Author

I think the CI failure should be unrelated to this PR.

@hogepodge
Copy link
Contributor

If you push a basic change it will re-kick the gate job. I’m sorry this is taking so long to merge.

@mkroening
Copy link
Contributor Author

Another spurious error, trying again.

@mkroening mkroening force-pushed the tutorial-fix branch 2 times, most recently from 95bb1cb to 75b0a24 Compare October 21, 2021 13:20
Martin Kröning added 4 commits October 21, 2021 18:12
@mkroening
Copy link
Contributor Author

Okay, on the sixth try CI passes. 🎉

@masahi masahi merged commit bb5e653 into apache:main Oct 23, 2021
@mkroening mkroening deleted the tutorial-fix branch October 23, 2021 00:18
@hogepodge
Copy link
Contributor

🎉

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
* tutorial: preprocess.py: Fix leading whitespace

This fixes the indentation of metadata in `preprocess.py` in the TVMC tutorial, removing the leading whitespaces in the HTML rendering[^1].

[^1] https://tvm.apache.org/docs/tutorial/tvmc_command_line_driver.html#preprocess-py

* tutorial: Add missing code block escapes

* tutorial: Grammar fixup

* README.md: Fix link to introduction

Co-authored-by: Martin Kröning <martin.kroening@neclab.eu>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* tutorial: preprocess.py: Fix leading whitespace

This fixes the indentation of metadata in `preprocess.py` in the TVMC tutorial, removing the leading whitespaces in the HTML rendering[^1].

[^1] https://tvm.apache.org/docs/tutorial/tvmc_command_line_driver.html#preprocess-py

* tutorial: Add missing code block escapes

* tutorial: Grammar fixup

* README.md: Fix link to introduction

Co-authored-by: Martin Kröning <martin.kroening@neclab.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: need update need update based on feedbacks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants