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

Develop merge examples #3851

Merged
merged 18 commits into from
Nov 27, 2023
Merged

Conversation

byrnHDF
Copy link
Contributor

@byrnHDF byrnHDF commented Nov 13, 2023

NOTE: goal is to have one folder with examples that can be distributed with the library source as well as be distributed with an installation/binary that can be used standalone with an installed hdf5.

CMake workflows integrated into library.
Autotools scripts updated with correct directory paths for reference files (not tested, not integrated with library configure)

@byrnHDF byrnHDF added Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - Build CMake, Autotools Type - Improvement Improvements that don't add a new feature or functionality Component - Misc Anything else (CODEOWNERS, etc.) labels Nov 13, 2023
@byrnHDF byrnHDF self-assigned this Nov 13, 2023
# Option to Enable MPI Parallel
#-----------------------------------------------------------------------------
set (CMAKE_MODULE_PATH ${H5EXAMPLES_RESOURCES_DIR} ${CMAKE_MODULE_PATH})
option (HDF_ENABLE_PARALLEL "Enable parallel build (requires MPI)" OFF)
Copy link
Collaborator

@jhendersonHDF jhendersonHDF Nov 14, 2023

Choose a reason for hiding this comment

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

Looks like this CMakeLists file needs to be updated to use the options that are already in the library (ENABLE_PARALLEL, ENABLE_THREADSAFE, BUILD_FORTRAN, BUILD_JAVA). Otherwise, we're bringing in duplicate options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, this would be true if I didn't also need to provide a standalone copy in the binaries that could could be used to verify the install as well as provide workable examples.

HDF5Examples/COPYING Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should a generated configure file be checked in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is because of the way the autotools were initially implemented. Someone will need to remove all the generated files and implement an autoconfig solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I add my examples, I'll see if I can make it look more like C's output. Are you ignoring white space and new lines when comparing the output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

newlines - definitely, Possibly not anything else.

@@ -733,6 +733,7 @@ EXCLUDE_PATTERNS += */hl/fortran/examples/*
EXCLUDE_PATTERNS += */hl/fortran/test/*
EXCLUDE_PATTERNS += */hl/fortran/src/*.c
EXCLUDE_PATTERNS += */hl/fortran/src/*.h
EXCLUDE_PATTERNS += */HDF5Examples/FORTRAN/H5T/*
Copy link
Contributor

Choose a reason for hiding this comment

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

You can exclude all files in */HDF5Examples/FORTRAN

@brtnfld
Copy link
Contributor

brtnfld commented Nov 21, 2023

We should rename the H5P directory to H5Parallel.

@byrnHDF
Copy link
Contributor Author

byrnHDF commented Nov 21, 2023

Unforyunately - no because I run into path lenght issues on some platforms and CI.

@byrnHDF byrnHDF closed this Nov 21, 2023
@byrnHDF byrnHDF reopened this Nov 21, 2023
@brtnfld
Copy link
Contributor

brtnfld commented Nov 21, 2023

I don't understand. Is that an HDF5 issue? Can we at least use H5PAR? I can look into it more if you are out of hours. I just need to get an idea of what to look into.

@byrnHDF
Copy link
Contributor Author

byrnHDF commented Nov 21, 2023

I don't understand. Is that an HDF5 issue? Can we at least use H5PAR? I can look into it more if you are out of hours. I just need to get an idea of what to look into.

Sure - two letters should be fine.

@lrknox lrknox merged commit fc88fcd into HDFGroup:develop Nov 27, 2023
45 checks passed
@byrnHDF byrnHDF deleted the develop-merge-examples branch December 19, 2023 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Component - Misc Anything else (CODEOWNERS, etc.) Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants