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.

Sign up for GitHub

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

Jump to bottom

Add API support for Fortran MPI_F08 module definitions. #3959

Merged
merged 10 commits into from
Jan 29, 2024
Merged

Add API support for Fortran MPI_F08 module definitions. #3959

merged 10 commits into from
Jan 29, 2024

Conversation

brtnfld
Copy link
Contributor

@brtnfld brtnfld commented Jan 24, 2024

Adds support for MPI's MPI_F08 module datatypes: type(MPI_COMM) and type(MPI_INFO) for HDF5 APIs:

H5PSET_FAPL_MPIO_F, H5PGET_FAPL_MPIO_F, H5PSET_MPI_PARAMS_F, H5PGET_MPI_PARAMS_F

Includes tests and updated documentation.

See: #3951

@brtnfld brtnfld changed the title Add API support for Fortran MPI_F08 module definitions Add API support for Fortran MPI_F08 module definitions. Jan 24, 2024
@brtnfld brtnfld added Merge - To 1.14 Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Component - Fortran Fortran wrappers Type - New Feature Add a new API call, functionality, or tool labels Jan 24, 2024
@brtnfld brtnfld added this to the 1.14.4 milestone Jan 24, 2024
epourmal
epourmal approved these changes Jan 25, 2024
View reviewed changes
Copy link

@epourmal epourmal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me. Question: is there any way to avoid code duplication in mpi_para.F90 using ifdef? The only difference is the type definition for info and comm variables, rigth?

@brtnfld brtnfld added the WIP Work in progress (please also convert PRs to draft PRs) label Jan 25, 2024
@brtnfld
Copy link
Contributor Author

brtnfld commented Jan 25, 2024

My assumption that TYPE(MPI*) is C interpretable is wrong, as it is for MPICH but not OpenMPI; I need to instead go back to using MPI_Comm_f2c, and I'll rework the test to simplify it.

ref. open-mpi/ompi#12274

@derobins derobins removed this from the 1.14.4 milestone Jan 25, 2024
@epourmal
Copy link

My assumption that TYPE(MPI*) is C interpretable is wrong, as it is for MPICH but not OpenMPI; I need to instead go back to using MPI_Comm_f2c, and I'll rework the test to simplify it.

ref. open-mpi/ompi#12274

I see. It would be really nice to get rid of the wrappers, but I guess, it will take time...

@brtnfld
Copy link
Contributor Author

brtnfld commented Jan 26, 2024

It's more straightforward to rewrite the test if I get into generic types (type(*)) or use classes; I'm not sure I want to introduce that complexity/Fortran standards into the tests.

@brtnfld brtnfld removed the WIP Work in progress (please also convert PRs to draft PRs) label Jan 26, 2024
@lrknox lrknox merged commit d79667e into HDFGroup:develop Jan 29, 2024
49 checks passed
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Feb 15, 2024
@brtnfld @lrknox
Add API support for Fortran MPI_F08 module definitions. (HDFGroup#3959)
30b0f89 
* revert to using c-stub for _F08 MPI APIs

* use mpi compiler wrappers for cmake and nvhpc
lrknox added a commit that referenced this pull request Feb 15, 2024
@lrknox
Merge pull request #4019 from lrknox/1_14_dev_sync2_lrk
413d10f 
* Update upload- artifact to match download version (#3929)

* Reorg and update options for doc and cmake config (#3934)

* Add binary build for linux S3 (#3936)

* Clean up Doxygen for szip functions and constants (#3943)

* Replace off_t with HDoff_t internally (#3944)

off_t is a 32-bit signed value on Windows, so we should use HDoff_t
(which is __int64 on Windows) internally instead.

Also defines HDftell on Windows to be _ftelli64().

* Fix chid_t to hid_t (#3948)

* Fortran API work. (#3941)

* - Added Fortran APIs:
      H5FGET_INTENT_F, H5SSELECT_ITER_CREATE_F, H5SSEL_ITER_GET_SEQ_LIST_F,
      H5SSELECT_ITER_CLOSE_F, H5S_mp_H5SSELECT_ITER_RESET_F

    - Added Fortran Parameters:
      H5S_SEL_ITER_GET_SEQ_LIST_SORTED_F, H5S_SEL_ITER_SHARE_WITH_DATASPACE_F

    - Added tests for new APIs
    - Removed H5F C wrapper stubs
    - Documentation misc. cleanup.

* Add the user test program in HDFFV-9174 for committed types. (#3937)

Add the user test program for committed types in HDFFV-9174

* Remove cached datatype conversion path table entries on file close (#3942)

* fixed BIND name (#3957)

* update H5Ssel_iter_reset_f test

* Change 'extensible' to 'fixed' in H5FA code (#3964)

* RF: move codespell configuration into .codespellrc so could be used locally as well (#3958)

* Add RELEASE.txt note for the fix for issue #1256 (#3955)

* Fix doxygen errors (#3962)

* Add API support for Fortran MPI_F08 module definitions. (#3959)

* revert to using c-stub for _F08 MPI APIs

* use mpi compiler wrappers for cmake and nvhpc

* Added a GitHub Codespaces configuration. (#3966)

* Fixed XL and gfortran errors (#3968)

* h5 compiler wrappers now pass all arguments passed to it to the compile line (#3954)

* The issue was that the "allargs" variable was not being used in the final command of the compiler wrapper. Any entries containing an escaped quote (\", \') or other non-matching argument (*) would not be passed to the compile line. I have fixed this problem by ensuring all arguments passed to the compiler wrapper are now included in the compile line.

* Add binary testing to CI testing (#3971)

* Replace 'T2' with ' ' to avoid failure to match expected output due to (#3975)

* Clarify vlen string datatype message (#3950)

* append '-WF,' when passing C preprocessor directives to the xlf compiler (#3976)

* Create CITATION.cff (#3927)

Add citation source based on http://web.archive.org/web/20230610185232/https://portal.hdfgroup.org/display/knowledge/How+do+I+properly+cite+HDF5%The space difference in the Fortran examples must be fixed to match the expected output for compression filter examples.

* corrected warning: implicit conversion changes signedness (#3982)

* Skip mac bintest until more reliable (#3983)

* Make platform specific test presets for windows and macs (#3988)

* chore: fix typo (#3989)

* Add a missing left parenthesis in RELEASE.txt. (#3990)

* Remove ADB signature from RELEASE.txt. (#3986)

* Bump the github-actions group with 6 updates (#3981)

* Sync API tests with vol-tests (#3940)

* Fix for github issue #2414: segfault when copying dataset with attrib… (#3967)

* Fix for github issue #2414: segfault when copying dataset with attributes.
This also fixes github issue #3241: segfault when copying dataset.
Need to set the location via H5T_set_loc() of the src datatype
when copying dense attributes.
Otherwise the vlen callbacks are not set up therefore causing seg fault
when doing H5T_convert() -> H5T__conv_vlen().

* Fix broken links caused by examples relocation. (#3995)

* Add abi-complience check and upload to releases (#3996)

* Fix h5watch test failures to ignore system warnings on ppc64le. (#3997)

* Remove oneapi/clang compiler printf() type warning. (#3994)

* Updated information about obtaining the HDF5 source code to use the repos. (#3972)

* Fix overwritten preset names (#4000)

* Fix incompatible pointer type warnings in object reference examples (#3999)

* Fix build issue and some warnings in H5_api_dataset_test.c (#3998)

* Modern C++ dtor declarations (#1830)

* C++ dtor modernization

- Replaced a bunch of empty dtors with `= default`
- Removed deprecated `throw()`. In C++11, dtors are `noexcept` by default.

* remove incorrect check for environ (#4002)

* Add a missing file into Makefile.am for MinGW Autotools build error. (#4004)

* Issue #1824: Replaced most remaining sprintf with safer snprint (#4003)

* Add hl and cpp ABI reports to daily build (#4006)

* Don't add files and directories with names that begin with ., or that match *autom4te* to release tar & zip files. (#4009)

* Fix some output issues with ph5diff (#4008)

* Update install texts (#4010)

* Add C in project line for CMake to fix #4012. (#4014)

* separate out individual checks for string removal (#4015)

* Add compound subset ops on attributes to API tests (#4005)

---------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byrnHDF byrnHDF byrnHDF approved these changes

@epourmal epourmal epourmal approved these changes

@derobins derobins Awaiting requested review from derobins derobins is a code owner

@lrknox lrknox Awaiting requested review from lrknox lrknox is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@jhendersonHDF jhendersonHDF Awaiting requested review from jhendersonHDF jhendersonHDF is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@mattjala mattjala Awaiting requested review from mattjala mattjala is a code owner

Assignees
No one assigned
Labels
Component - Fortran Fortran wrappers Priority - 0. Blocker ⛔ This MUST be merged for the release to happen Type - New Feature Add a new API call, functionality, or tool
Projects
Downstream HDF5 Merges
Status: Needs Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@brtnfld @epourmal @byrnHDF @derobins @lrknox