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

MPI Window #1

Open
wants to merge 47 commits into
base: shm
Choose a base branch
from
Open

MPI Window #1

wants to merge 47 commits into from

Conversation

Mobellaaj
Copy link

No description provided.

Thoemi09 and others added 30 commits September 11, 2024 17:13
Copy link
Owner

@hmenke hmenke left a comment

Choose a reason for hiding this comment

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

Please also have a look at the comments in TRIQS#14

@@ -36,6 +38,8 @@ namespace mpi {
class window {
friend class shared_window<BaseType>;
MPI_Win win{MPI_WIN_NULL};
BaseType* base_local{nullptr};
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
BaseType* base_local{nullptr};
public:
std::span<BaseType> base;

MPI_Datatype target_datatype = mpi_type<TargetType>::get();
MPI_Get(origin_addr, origin_count, origin_datatype, target_rank, target_disp, target_count_, target_datatype, win);
} else {
if ((sizeof(OriginType) * origin_count) >= (sizeof(TargetType) * target_count_)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if ((sizeof(OriginType) * origin_count) >= (sizeof(TargetType) * target_count_)) {
if ((sizeof(OriginType) * origin_count) >= (sizeof(TargetType) * target_count_) + target_disp) {

Assumes that target_disp is in bytes. (Check with the standard maybe?)

if ((sizeof(OriginType) * origin_count) >= (sizeof(TargetType) * target_count_)) {
std::memcpy(origin_addr, base_local + target_disp, sizeof(TargetType) * target_count_);
} else {
std::abort(); // Not enough memory
Copy link
Owner

Choose a reason for hiding this comment

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

Remove noexcept and throw std::bad_alloc.

Comment on lines 201 to 225
switch (op) {
case MPI_SUM:
for (int i = 0; i < origin_count; ++i) {
base_local[target_disp + i] += origin_addr[i];
}
break;
case MPI_PROD:
for (int i = 0; i < origin_count; ++i) {
base_local[target_disp + i] *= origin_addr[i];
}
break;
case MPI_MIN:
for (int i = 0; i < origin_count; ++i) {
base_local[target_disp + i] = std::min(base_local[target_disp + i], origin_addr[i]);
}
break;
case MPI_MAX:
for (int i = 0; i < origin_count; ++i) {
base_local[target_disp + i] = std::max(base_local[target_disp + i], origin_addr[i]);
}
break;
default:
std::cerr << "Error: Unsupported operation." << std::endl;
std::abort();
}
Copy link
Owner

Choose a reason for hiding this comment

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

For now just std::abort. We have to think of a usable solution. Maybe have a look at what all_reduce does without has_env.

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.

4 participants