-
Notifications
You must be signed in to change notification settings - Fork 54
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
function for element-wise operations with coordinate index #452
base: master
Are you sure you want to change the base?
Conversation
…istArray objects that keeps track of the coordinate index for conditional operations.
@Marclie thanks for the contribution! We need to address several issues first:
I'm not actually sure if the proposed vector<double> vec(n*n*n);
std::generate(v.begin(), v.end(), std::rand);
forall(array, [&vec] (auto& tile, auto& index) {
size_t i = index[0], j = index[1], k = index[2];
if (i <= j && j <= k) {
tile[index] = std::sqrt(vec[i*n*n+j*n+k]);
} else {
tile[index] = 0.0;
}
}); is any shorter than this vector<double> vec(n*n*n);
std::generate(v.begin(), v.end(), std::rand);
foreach_inplace(array, [&vec] (auto& tile) {
for(auto& idx: tile.range()) {
size_t i = idx[0], j = idx[1], k = idx[2];
if (i <= j && j <= k) {
tile[idx] = std::sqrt(vec[i*n*n+j*n+k]);
} else {
tile[idx] = 0.0;
}
}
}); If you think there is a value to element wise operations we need to extend the P.S. See also the implementation of |
… based on the traits of Op.
# Conflicts: # src/TiledArray/conversions/foreach.h
@evaleev Thank you for your review! I did not realize we could directly iterate over the tile ranges like that. Useful! We have been using However, I do agree that my wrapper is no more cleaner than the syntax for direct iteration over the range, and it may not add anything more substantial. Perhaps explicitly restricting the type traits of Once more, the tile iteration is extremely useful to know. I am happy with closing my pull request knowing this functionality already exists in a clean syntax, and I'll be refactoring my projects with it. |
@Marclie I think the template parameter constraints are useful, I'm certainly happy to merge them in ... re element wise ops: I'm currently on the fence about them, for the following reasons:
I agree that newbies want to use element-wise operations, and for constructing a |
Adds a function
forall
that behaves likeforeach_inplace
, but also passes in the coordinate index with each tile. I've used this code in my projects with tiledarray to cleanly extract, fill, and modify the upper diagonal elements of a tiledarray object.The function uses
foreach_inplace
and a recursive loop for iteration. The recursion helps the user focus on element-wise operations without explicitly creating loops for each dimension that use thelobound
andupbound
of each tile.I added a test in
tests/foreach.cpp
to show its usage and ensure it works as intended. If you find this feature useful and everything looks good, feel free to merge. If this feature already exists elsewhere, please let me know. Thank you for your consideration!