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

Deprecate MPI without MPI I/O #171

Merged

Conversation

tim-griesbach
Copy link
Collaborator

Deprecate MPI without MPI I/O

Proposed changes:

  • Deprecate the configuration with MPI and without MPI I/O.
  • Avoid using MPI link-time constants as compile-time constants in non-deprecated configuration setups.
  • Exclude the case of MPI without MPI I/O from the documentation in sc_io.h.
  • Remove sc_io_{write,read}_at_legal since they are only related to the deprecated configuration setup.
  • Remove sc_io_{write,read}_all since it is unused and not conforming to the corresponding MPI_File functions in any configuration setup.
  • Update the release notes for the changes in this PR, change existing release notes concerning MPI without MPI I/O and adding a note for the PR Make sc_io_open independent of the MPI IO status #130.

This draft PR would also profit from the changes in @cburstedde's branch https://github.com/cburstedde/libsc/tree/feature-disable-mpiio introducing warnings if the deprecated configuration setup is used.

Moreover, this draft PR relates to the issue #170 by not using MPI_ERR_LASTCODE in non-deprecated configuration setups.

@cburstedde
Copy link
Owner

Thanks. Will it be reasonable to integrate #172 first?

@tim-griesbach
Copy link
Collaborator Author

Thanks. Will it be reasonable to integrate #172 first?

Yes, I think it makes sense to integrate the PR #172 first.

src/sc_scda.h Outdated Show resolved Hide resolved
@cburstedde
Copy link
Owner

Thanks! I'm just wondering whether the scda feature should do full error checking both with and without MPI. Is it necessary to document that we may abort? This limits community use.

@cburstedde cburstedde marked this pull request as ready for review March 8, 2024 15:36
@tim-griesbach
Copy link
Collaborator Author

Thanks! I'm just wondering whether the scda feature should do full error checking both with and without MPI. Is it necessary to document that we may abort? This limits community use.

This is a very good point! I rechecked the code and without the deprecated case of MPI without MPI I/O we only have one file functions related abort. It is just a simple consistency check in sc_io_close.

    errno = 0;
    retval = fclose ((*mpifile)->file);
    mpiret = sc_io_error_class (errno, &eclass);
    SC_CHECK_MPI (mpiret);
    SC_CHECK_ABORT (!retval == (eclass == sc_MPI_SUCCESS),
                    "fclose return value inconsistent");

Therefore, we may want to remove the comments on the file system dependent aborts since there will be no aborts beyond simple consistency checks.

src/sc_scda.h Outdated Show resolved Hide resolved
@cburstedde
Copy link
Owner

cburstedde commented Mar 8, 2024

Thanks! This might be all for this PR.

When continuing working with SCDA, we may comment on other error conditions that may occur, mostly errors in the file contents encountered on reading.

@cburstedde
Copy link
Owner

Are you generally happy with the state of our doxygen output?

@tim-griesbach
Copy link
Collaborator Author

Are you generally happy with the state of our doxygen output?

The doxygen output looks good to me. I adjusted the I/O page since it was still referring to the deprecated case with MPI but without MPI I/O.

@cburstedde
Copy link
Owner

Cool let's see how this flies with the community once p4est updates its submodule.

@cburstedde cburstedde merged commit 4b25600 into cburstedde:develop Mar 8, 2024
17 checks passed
@tim-griesbach tim-griesbach deleted the feature-doc-disable-mpiio branch March 8, 2024 20:23
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