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

Implementation for mpio driver with selection I/O. #3058

Closed
wants to merge 46 commits into from

Conversation

vchoi-hdfgroup
Copy link
Contributor

This is a draft for preliminary review.

@vchoi-hdfgroup vchoi-hdfgroup added the WIP Work in progress (please also convert PRs to draft PRs) label Jun 6, 2023
@vchoi-hdfgroup vchoi-hdfgroup added the Component - C Library Core C library issues (usually in the src directory) label Jun 6, 2023
@derobins derobins added Merge - To 1.14 Priority - 1. High 🔼 These are important issues that should be resolved in the next release labels Jun 6, 2023
src/H5FDint.c Outdated
@@ -1555,7 +1562,7 @@ H5FD__write_selection_translate(H5FD_t *file, H5FD_mem_t type, hid_t dxpl_id, ui
io_len = MIN(file_len[file_seq_i], mem_len[mem_seq_i]);

/* Check if we're using vector I/O */
if (use_vector) {
if (!skip_vector_cb && use_vector) {
Copy link
Member

Choose a reason for hiding this comment

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

Should use skip_vector_cb to determine the value of use_vector on line 1460, then there will be no need to keep checking it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/H5FDint.c Outdated
*-------------------------------------------------------------------------
*/
herr_t
H5FD_selection_build_types(hbool_t io_op_write, uint32_t num_pieces, H5S_t **file_spaces, H5S_t **mem_spaces,
Copy link
Member

Choose a reason for hiding this comment

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

Having this function in a shared place is a little strange. I was originally thinking that the code in H5FDmpio.c and H5Dmpio.c shouldn't be shared. If it's working though it might be worth it. I'll continue thinking about a way to make this function more self-contained/generic/maintainable

Copy link
Member

Choose a reason for hiding this comment

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

After further thought, I think we should move this function to H5FDmpio.c, and return this functionality to being integrated into link_piece_collective_io (with the frees moved to before the I/O as suggested in my email). I think it'd also be a good idea to move the check for num_pieces out of the function (and into the read/write callbacks), and remove the size_i and mpi_buds_base parameters (calculate them in the read/write_callbacks).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will move this function to H5FDmpio.c as indicated.
And I will restore link_piece_collective_io() back to its original state in H5Dmpio.c (with the frees moved to before the IO), is that correct?

Copy link
Member

Choose a reason for hiding this comment

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

That's correct, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes made as indicated for H5FD_selection_build_types().
However, I still need to pass num_pieces to the routine because it is needed to malloc the arrays of mtype, ftype and displacement etc. I can calculate mpi_bufs_base in the read/write callbacks, but I still need to pass the calculated info to H5FD_selection_build_types() for calculating displacements of the pieces.

github-actions bot and others added 25 commits June 27, 2023 01:05
….c when selection I/O is ON by default in the library.

a) Create internal functions H5FD_read/write_vector_from_selection() and H5FD_read/write_from_selection() that correspond to the 4 new APIs H5FDread/write_vector_from_selection() and H5FDread/write_from_selection().
--Call the internal functions H5FD_read/write_vector_from_selection in H5FD__mpio_read/write_selection() for independent xfer_mode.
when selection I/O is ON by default in the library.
--Undo the base address addition in internal routines before passing down to the mpio driver
…/t_dsets.c (via testpar/testphdf5.c)

when selection I/O is ON by default in the library:
--need to check for fixed bufs when trying to figure out mpi_bufs_base in H5FD__selection_build_types().
This is the fix for a failure from test_link_chunk_io_sort_chunk_issue() test in
testpar/t_coll_md.c (via testpar/testphdf5.c) when selection I/O is ON by default
in the library:
--need to check for fixed bufs when trying to figure out mpi_bufs_base in H5FD__selection_build_types().
system() is only used in the iopipe test and the things it calls
(which are POSIX-y) are protected by an ifdef.
@vchoi-hdfgroup
Copy link
Contributor Author

I am going to create a new PR based on the latest "develop" branch and incorporate the mpio driver changes to there. I think it's easier that way instead of resolving all the conflicts/CI failures etc. Then I will close this PR.

vchoi-hdfgroup added a commit to vchoi-hdfgroup/hdf5 that referenced this pull request Jul 3, 2023
This also addresses the review comments of the previous PR HDFGroup#3058 which will be closed.
@vchoi-hdfgroup
Copy link
Contributor Author

See PR #3222

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - C Library Core C library issues (usually in the src directory) Priority - 1. High 🔼 These are important issues that should be resolved in the next release WIP Work in progress (please also convert PRs to draft PRs)
Projects
Status: Merges Complete
Development

Successfully merging this pull request may close these issues.

4 participants