-
Notifications
You must be signed in to change notification settings - Fork 90
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
Add local to global index mapping #1707
base: develop
Are you sure you want to change the base?
Conversation
1070d82
to
8392d07
Compare
8392d07
to
a6e1366
Compare
c43c48a
to
c3b27c2
Compare
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 finish review yet.
why is it required for PGM?
after merging row and column, do they still need to map back to the global index?
you need to reindex all of them because the size will be the smaller than the original one after coarsening.
// | ||
// SPDX-License-Identifier: BSD-3-Clause | ||
|
||
#ifndef GINKGO_PARTITION_HPP |
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.
nit:
#ifndef GINKGO_PARTITION_HPP | |
#ifndef GKO_CORE_DISTRIBUTED_DEVICE_PARTITION_HPP_ |
/** | ||
* Create device_segmented_array from a segmented_array. | ||
*/ |
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.
forgot to update documentation?
* Explicitly create a const version of device_segmented_array. | ||
* | ||
* This is mostly relevant for tests. |
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.
same here
*/ | ||
template <typename LocalIndexType, typename GlobalIndexType> | ||
constexpr device_partition<const LocalIndexType, const GlobalIndexType> | ||
to_device_const( |
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.
is there any difference between to_device_const
and to_device
?
* | ||
* The relevant input parameter from the index map are: | ||
* | ||
* - partition: the global partition |
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.
* - partition: the global partition | |
* - partition: the global partition |
}); | ||
|
||
sizes.resize_and_reset(num_parts); | ||
std::fill_n(sizes.get_data(), num_parts, int64(0)); |
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.
std::fill_n(sizes.get_data(), num_parts, int64(0)); | |
std::fill_n(sizes.get_data(), num_parts, zero<int64>()); |
auto seed = engine_dist(rd); | ||
std::cout << "seed = " << seed << std::endl; | ||
engine.seed(490729788); |
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.
some leftover?
exec->run(partition::make_build_ranges_by_part( | ||
part_ids_.get_const_data(), get_num_ranges(), get_num_parts(), | ||
range_ids, num_ranges_per_part)); | ||
ranges_by_part_ = segmented_array<size_type>::create_from_sizes( | ||
std::move(range_ids), num_ranges_per_part); |
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.
currently, this rely on the atomic add to count the size.
then segmented_array will convert size to offset.
Maybe you can implement a similar kernel with convert_idxs_to_ptrs + additional index map from i to range_id[i] because range_part[range_id[i]]
is sorted. then segmented_array can create_from_offsets
directly
template <typename LocalIndexType, typename GlobalIndexType> | ||
GlobalIndexType map_to_global( | ||
LocalIndexType idx, | ||
device_partition<const LocalIndexType, const GlobalIndexType> partition, |
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.
device_partition<const LocalIndexType, const GlobalIndexType> partition, | |
const device_partition<const LocalIndexType, const GlobalIndexType>& partition, |
device_partition<const LocalIndexType, const GlobalIndexType> partition, | ||
size_type range_id) | ||
{ | ||
auto range_bounds = partition.offsets_begin; |
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.
assert range_id is lower than num_range?
@yhmtsai I don't understand your point, could you rephrase this? |
rank 0: holds row 0, 1, 4 |
yes, but if PGM does not need it, do you still need this function for something else? |
I think I need at least parts of it to keep the old distributed matrix constructor around. @fritzgoebel also seems to need it for his domain decomposition matrix. Anyway, this still sounds like a discussion for #1639. |
This PR adds a local-to-global mapping to the
index_map
class. It is necessary for the follow-up PR #1639. This also introduces a device-side view of a partition, and adds another member to the partition which stores the range-ids segmented by their part-id.(I know that the branch name is wrong...)