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.

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

[doc] Minor updates in hello.rst #963

Merged
merged 3 commits into from
May 13, 2020
Merged

Conversation

yuanming-hu
Copy link
Member

Related issue = #941

(Found these issues while translating the doc to Chinese)

[Click here for the format server]

@yuanming-hu yuanming-hu requested a review from archibate May 12, 2020 19:21
@yuanming-hu yuanming-hu requested a review from TH3CHARLie May 13, 2020 04:43
@yuanming-hu
Copy link
Member Author

@TH3CHARLie thanks for the tips. Although the code samples in Interacting with Python is not part of fractal.py, I still think it is helpful to mention these features here. I made minor modifications to make the transition smoother.

@TH3CHARLie
Copy link
Collaborator

LGTM! The new code samples added in Interacting with Python makes this a complete and great introduction now!

Copy link
Collaborator

@archibate archibate left a comment

Choose a reason for hiding this comment

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

Thank for refactoring the doc, LGTM except for a few nits.

@@ -129,8 +129,8 @@ You can also define Taichi **functions** with ``ti.func``, which can be called a

.. note::

**Taichi-scope v.s. Python-scope**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scope, which will be compiled by the Taichi compiler.
Code outside the Taichi-scopes is simply normal Python code.
**Taichi-scopes v.s. Python-scopes**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scope, which will be compiled by the Taichi compiler.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want everything to be scopes, then why not:

Suggested change
**Taichi-scopes v.s. Python-scopes**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scope, which will be compiled by the Taichi compiler.
**Taichi-scopes v.s. Python-scopes**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scopes, which will be compiled by the Taichi compiler.

You may want to run find docs -exec grep -Hw scope {} \; to find out more.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's unify the usage of scope in a separate PR to make this one easier to review :-)

**Taichi-scope v.s. Python-scope**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scope, which will be compiled by the Taichi compiler.
Code outside the Taichi-scopes is simply normal Python code.
**Taichi-scopes v.s. Python-scopes**: everything decorated with ``ti.kernel`` and ``ti.func`` is in Taichi-scope, which will be compiled by the Taichi compiler.
Everything else is in Python-scopes. They are simply Python code.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still feel this a bit unconfortable..

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't read good to me either, but at least it's grammatically correct. Let's tolerate these places for now (unless there is a solution that is clearly better). Hopefully, someone native in English can help us out in the future.

@yuanming-hu
Copy link
Member Author

Merging this for v0.6.4. As promised I'll unify the terminologies later: #941 (comment)

@yuanming-hu yuanming-hu merged commit f503dc4 into taichi-dev:master May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants