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

[refactor] Cleanup python imports #2226

Merged
merged 1 commit into from
Mar 22, 2021
Merged

[refactor] Cleanup python imports #2226

merged 1 commit into from
Mar 22, 2021

Conversation

k-ye
Copy link
Member

@k-ye k-ye commented Mar 21, 2021

@k-ye k-ye requested a review from yuanming-hu March 21, 2021 09:35
python/taichi/lang/type_factory_impl.py Show resolved Hide resolved
python/taichi/lang/kernel.py Show resolved Hide resolved
# object within it, is that ti_core is stateful. While in practice ti_core is
# loaded during the import procedure, it's probably still good to delay the
# access to it.
from taichi.core import util as cutil
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably do this for all the other aliases of ti_core as well?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good :-)



@unit('task')
def _unit(unit_name):
Copy link
Member Author

Choose a reason for hiding this comment

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

Does this mean unittest?

Copy link
Member

Choose a reason for hiding this comment

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

unit was a wrapper over the legacy interface/impl-managing system used in legacy Taichi. I think there are still usages of unit. task is an example: https://taichi.readthedocs.io/en/stable/contributor_guide.html?highlight=libdevice#upgrading-cuda

The interface/unit system is intended to improve the modularity of the legacy Taichi library. I think we do not need them anymore. Given it seems overdesigned at this point, we may need to at least simplify it...

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see.

task is an example:

Right. I searched for @unit and tasks is the only user, so I moved unit here (instead of staying at core)..

python/taichi/misc/util.py Show resolved Hide resolved
Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

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

Looks great! Sorry about leaving these messy imports for you. It's clearly a lot of work to clean these up - thank you so much!

python/taichi/misc/util.py Show resolved Hide resolved


@unit('task')
def _unit(unit_name):
Copy link
Member

Choose a reason for hiding this comment

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

unit was a wrapper over the legacy interface/impl-managing system used in legacy Taichi. I think there are still usages of unit. task is an example: https://taichi.readthedocs.io/en/stable/contributor_guide.html?highlight=libdevice#upgrading-cuda

The interface/unit system is intended to improve the modularity of the legacy Taichi library. I think we do not need them anymore. Given it seems overdesigned at this point, we may need to at least simplify it...

# object within it, is that ti_core is stateful. While in practice ti_core is
# loaded during the import procedure, it's probably still good to delay the
# access to it.
from taichi.core import util as cutil
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good :-)

@k-ye k-ye merged commit adfe729 into taichi-dev:master Mar 22, 2021
@k-ye k-ye deleted the fix-py2 branch March 22, 2021 04:14
@xumingkuan xumingkuan mentioned this pull request Apr 13, 2021
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.

2 participants