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

Allow parallel I/O for hydro and particle reading #4730

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

cphyc
Copy link
Member

@cphyc cphyc commented Nov 3, 2023

PR Summary

This allows parallel I/O for the RAMSES dataset. Reading a RAMSES dataset has three steps:

  • read in the AMR structure
  • read the hydro files
  • read the particle files

The strategy I have adopted is to parallelize on the first one, such that each MPI task is now in charge of a few domains and only reads those (incl. hydro + particles).

@cphyc cphyc force-pushed the parallelize-ramses-io branch from 4b482d2 to ce2aa15 Compare November 3, 2023 11:21
@cphyc cphyc force-pushed the parallelize-ramses-io branch from ce2aa15 to a8d7767 Compare November 3, 2023 11:30
@@ -1157,9 +1199,9 @@ def alltoallv_array(self, send, total_size, offsets, sizes):
dtr = send.dtype.itemsize / tmp_send.dtype.itemsize # > 1
roff = [off * dtr for off in offsets]
rsize = [siz * dtr for siz in sizes]
tmp_recv = recv.view(self.__tocast)
tmp_recv = recv.view(transport_dtype)
Copy link
Member Author

Choose a reason for hiding this comment

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

The motivation for getting rid of the cast to CHAR is so that we do not hit the limit imposed by MPI of 2**31 elements when communicating arrays as quickly.

Comment on lines +1153 to +1158
self.comm.send(
(arr.dtype.str, arr.shape, transport_dtype) + unit_metadata,
dest=dest,
tag=tag,
)
self.comm.Send([arr, mpi_type], dest=dest, tag=tag)
Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't there a bug here? I think this should be

Suggested change
self.comm.send(
(arr.dtype.str, arr.shape, transport_dtype) + unit_metadata,
dest=dest,
tag=tag,
)
self.comm.Send([arr, mpi_type], dest=dest, tag=tag)
self.comm.send(
(arr.dtype.str, arr.shape, transport_dtype) + unit_metadata,
dest=dest,
tag=tag,
)
self.comm.Send([tmp, mpi_type], dest=dest, tag=tag)

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be tmp, yeah, but since it has been working I think it's possible that mpi4py was fixing it implicitly.

@cphyc
Copy link
Member Author

cphyc commented Nov 3, 2023

@yt-fido test this please

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.

2 participants