-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[lang] Return None when parent() exceeds its maximum depth #1319
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1319 +/- ##
==========================================
+ Coverage 66.41% 66.49% +0.08%
==========================================
Files 37 37
Lines 5133 5146 +13
Branches 931 932 +1
==========================================
+ Hits 3409 3422 +13
Misses 1562 1562
Partials 162 162
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a quick look, lgtmig!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! LGTM.
Offtopic discussion: maybe we should make |
Agree. |
Actually, probably not. I think taichi/python/taichi/lang/impl.py Lines 250 to 253 in 13462ff
While we can say one should only access As a side effect, I found it useless to define |
Good point. How about just setting |
Allow me to merge this one first. I've encountered some problems when modifying global |
Ah you are right... I think that's the reason why I did a |
Seems like https://github.com/taichi-dev/taichi/pull/1214/files#r440768343 wasn't addressed. I'd suggest us not to make root circular and keep the parent chain linear. Following parent until null is a fairly common op, e.g.
taichi/taichi/transforms/offload.cpp
Lines 87 to 95 in 37976e7
This PR also handles the case where
n
exceeds the hierarchy depth.Related issue = #1214
[Click here for the format server]