-
Notifications
You must be signed in to change notification settings - Fork 536
Split delinearize_index out of broadcast_util #9056
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
Conversation
void delinearize_index( | ||
size_t linear_index, | ||
executorch::aten::ArrayRef<Tensor::SizesType> shape, | ||
size_t* out_indexes, |
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.
we have Span fwiw.
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.
Not particularly interested in changing API here because 1) out of scope for this diff 2) might break backward compatibility 3) don't see a reason to migrate between the two equivalent reference-to-array classes.
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.
- don't see a reason to migrate between the two equivalent reference-to-array classes.
Ah I meant more for the bottom 2 args of * and length not the arrayref
project(extension_parallel_test) | ||
|
||
# Use C++17 for test. | ||
set(CMAKE_CXX_STANDARD 17) |
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.
Why'd the standard line go away?
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.
we don't need it
} // namespace executor | ||
} // namespace torch | ||
// This header is a stub left behind after the move to | ||
// executorch/runtime/kernel. As such, it is deprecated; include and |
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.
were any of our tutorials using thread_parallel.h?
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.
code search says no
Left an #include behind for backward compatibility. Just needed so that broadcast_indexes_range.h can use delinearize_index