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

Issue96 #110

Merged
merged 5 commits into from
Feb 8, 2021
Merged

Issue96 #110

merged 5 commits into from
Feb 8, 2021

Conversation

gvallee
Copy link
Owner

@gvallee gvallee commented Feb 6, 2021

Fix #96 including a bug fix to correctly report the datatype size.

@gvallee gvallee added the WIP Work in progress label Feb 6, 2021
{
fprintf(stderr, "[%s:%d][ERROR] unable to insert send/recv counts\n", __FILE__, __LINE__);
MPI_Abort(MPI_COMM_WORLD, 1);
}
#endif // ((ENABLE_RAW_DATA || ENABLE_PER_RANK_STATS || ENABLE_VALIDATION) && ENABLE_COMPACT_FORMAT)

#if ((ENABLE_RAW_DATA || ENABLE_PER_RANK_STATS || ENABLE_VALIDATION) && !ENABLE_COMPACT_FORMAT)
save_counts(sbuf, rbuf, sizeof(sendtype), sizeof(recvtype), comm_size, avCalls);
int s_dt_size, r_dt_size;
MPI_Type_size(sendtype, &s_dt_size);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the same as for alltoall.c, which may of course be what you want. For alltoall it is not necesary to gather the send types because the counts are gathered and bocksize = sendtypesize * sendcount is a constant, so having gathered the sendcounts the sendtypesize used at each node can be inferred. Does something similar apply to the more complicated alltollv - I have not worked it out yet but man for alltoallv says this, which may be helpful:

"When a pair of processes exchanges data, each may pass different element count and datatype arguments so long as the sender specifies the same amount of data to send (in bytes) as the receiver expects to receive.

"Note that process i may send a different amount of data to process j than it receives from process j. Also, a process may send entirely different amounts of data to different processes in the communicator. "

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think I am missing something:

  • i did the same change for alltoall
  • this issue is not about collecting the datatypes, it is about fixing a problem with the current implementation. If we decide to collect all the datatypes (which i personally think should be done), it should be a separate issue; not this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree it should be a separate issue. Just thought it worth a precautionary heads up before you work on the golang consumer of this data. Observation - the github interface is not so good a drawing attention to motivation for and decisions on scope of changes - or am I still a newbie looking in the wrong place?!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Issue #111

@gvallee gvallee removed the WIP Work in progress label Feb 8, 2021
@gvallee gvallee merged commit a87ccb5 into master Feb 8, 2021
@gvallee gvallee deleted the issue96 branch February 8, 2021 07:20
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.

Write a validation test for datatype sizes
2 participants