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

New3rd party libraries #3110

Merged
merged 35 commits into from
Aug 17, 2022
Merged

New3rd party libraries #3110

merged 35 commits into from
Aug 17, 2022

Conversation

sgpearse
Copy link
Collaborator

@sgpearse sgpearse commented May 25, 2022

This PR updates Vapor to use ZFP enabled HDF5 and an updated version of Ospray. Users must set the HDF5_PLUGIN_PATH to Vapor's /lib directory to allow for ZFP enabled compression.

Since not all libraries were rebuilt, the newly built libraries had to be targeted at /usr/local/VAPOR-Deps/2019-Aug again.

The new libraries can be found on google drive. They have been named 2019-Aug-[OS]-2022.tar.xz

Links here:
Ubuntu
CentOS
OSX
Windows

d4810cb: clangTidyOutput.txt

Fixes #3179
Fixes #3158
Fixes #3174
Fixes #2816

@shaomeng
Copy link
Collaborator

shaomeng commented May 31, 2022

Hi Scott, do you mind updating the glm library too? It's a header-only library so should be easy to deal with, yet updating it is aligned with our ultimate goal. (https://github.com/g-truc/glm)

@clyne clyne requested review from StasJ and shaomeng June 1, 2022 20:31
@clyne
Copy link
Collaborator

clyne commented Jun 1, 2022

@sgpearse can you please identify which libraries were updated and their version numbers?

Copy link
Collaborator

@StasJ StasJ left a comment

Choose a reason for hiding this comment

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

Is there a reason we will require users to manually set HDF5_PLUGIN_PATH to enable ZFP?

@sgpearse
Copy link
Collaborator Author

@ALL - HDF5 seems to be working on Windows now. I'll request re-reviews once I can verify that the build systems are referencing the new libraries. Answers to your questions are below.

Hi Scott, do you mind updating the glm library too?

@shaomeng - No problem.

@sgpearse can you please identify which libraries were updated and their version numbers?

@clyne
HDF5 - 1.12.2
HDF5 Plugins - 1.12.2
NetCDF - 4.8.1
Ospray - 2.9.0
GLM - 0.9.9.8

Is there a reason we will require users to manually set HDF5_PLUGIN_PATH to enable ZFP?

@StasJ - From what I've read, this is necessary. From the NetCDF documentation:

At the moment, NetCDF uses the existing HDF5 environment variable HDF5_PLUGIN_PATH to locate the directories in which filter wrapper shared libraries are located. This is used both for the HDF5 filter wrappers but also the NCZarr codec wrappers.

There may be a way to use HDF5's API for this but I'm not sure what it entails.

@sgpearse
Copy link
Collaborator Author

Installers have been verified on all platforms with the new libraries. If you'd like to try the installers, you can find them in the artifacts on these builds.

@StasJ I was able to remove the need for HDF5_PLUGIN_PATH on our UNIX systems. I had no luck building SZlib+HDF5+NetCDF together, so I used Unidata's NetCDF binary installer for Windows. This leaves us without the HDF5 library needed to make the programmatic call for finding the plugin path.

@StasJ and @clyne - the programatic call for setting the HDF5 plugin path is currently in apps/vaporgui/main.cpp. Should this be moved elsewhere?

@sgpearse sgpearse requested a review from StasJ June 25, 2022 00:33
@clyne
Copy link
Collaborator

clyne commented Jun 27, 2022

@sgpearse it looks to me like you are calling H5PLreplace() on Mac and Linux platforms, not windows. I'm a little confused because I thought you only need to set the plug path explicitly on Windows systems?

Regardless, setting the path in the vaporgui app. is probably not ideal; our conversion utilities won't work. Any thoughts on this @StasJ? I'm not sure if it is a disaster if we can't convert to/from hdf5 to VDC on Windows platforms. We may just have to live with that.

@shaomeng
Copy link
Collaborator

@sgpearse it looks to me like you are calling H5PLreplace() on Mac and Linux platforms, not windows. I'm a little confused because I thought you only need to set the plug path explicitly on Windows systems?

Regardless, setting the path in the vaporgui app. is probably not ideal; our conversion utilities won't work. Any thoughts on this @StasJ? I'm not sure if it is a disaster if we can't convert to/from hdf5 to VDC on Windows platforms. We may just have to live with that.

Here's an idea: in case that we have to distribute VAPOR with different capabilities among 3 major platforms, or even capabilities that are easy to go wrong within the camp of Linux for example, we provide a utility program that detects those variables and report to the user. It could be some added fields to "vaporversion".
The following capabilities can be subject to detection and report:

  1. Whether ZFP (or any other) compression is supported
  2. Whether OpenMP is supported
  3. On tricky grids, whether GPU accelerated DVR is supported

Copy link
Collaborator

@StasJ StasJ left a comment

Choose a reason for hiding this comment

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

@sgpearse it looks to me like you are calling H5PLreplace() on Mac and Linux platforms, not windows. I'm a little confused because I thought you only need to set the plug path explicitly on Windows systems?

Regardless, setting the path in the vaporgui app. is probably not ideal; our conversion utilities won't work. Any thoughts on this @StasJ? I'm not sure if it is a disaster if we can't convert to/from hdf5 to VDC on Windows platforms. We may just have to live with that.

Couldn’t we just set the plugin path in the VDC API when attempting to load an HDF5 dataset? We could keep track of whether it had been set or the HDF5 API allows you to query the existing plugin paths.

@clyne
Copy link
Collaborator

clyne commented Jun 29, 2022

Couldn’t we just set the plugin path in the VDC API when attempting to load an HDF5 dataset? We could keep track of whether it had been set or the HDF5 API allows you to query the existing plugin paths.

That could be done, but to ensure all of our readers and writers that access NetCDF files (e.g. DCMPAS, DCWRF, VDNetCDF, etc.) are supported we would have to do this in multiple places; there is not one single place where NetCDF is initialized, unfortunately.

Another option might be to not touch the code, and to instead set the HDF5_PLUGIN_PATH. It looks like HDF5 will honor that. I don't what is involved in setting an env variable for our Windows installer.

@clyne
Copy link
Collaborator

clyne commented Jun 29, 2022

Here's an idea: in case that we have to distribute VAPOR with different capabilities among 3 major platforms, or even capabilities that are easy to go wrong within the camp of Linux for example, we provide a utility program that detects those variables and report to the user. It could be some added fields to "vaporversion".
The following capabilities can be subject to detection and report:
Whether ZFP (or any other) compression is supported
Whether OpenMP is supported
On tricky grids, whether GPU accelerated DVR is supported

Good idea, @shaomeng. Could you open an issue?

@shaomeng
Copy link
Collaborator

Good idea, @shaomeng. Could you open an issue?

Sure. #3142 is opened now.

@clyne
Copy link
Collaborator

clyne commented Jun 29, 2022

@sgpearse we need seem clarification. Does the HDF5 plugin path need to be explicitly set on all platforms?

@sgpearse
Copy link
Collaborator Author

sgpearse commented Jul 5, 2022

@sgpearse we need seem clarification. Does the HDF5 plugin path need to be explicitly set on all platforms?

HDF5_PLUGIN_PATH only needs to be set on Windows. It also needs to be set on our personal development machines because GetResourcePath() only works on installed versions of Vapor. It does not return /usr/local/VAPOR-Deps/2019-Aug during development builds.

@StasJ
Copy link
Collaborator

StasJ commented Jul 5, 2022

@sgpearse we need seem clarification. Does the HDF5 plugin path need to be explicitly set on all platforms?

HDF5_PLUGIN_PATH only needs to be set on Windows. It also needs to be set on our personal development machines because GetResourcePath() only works on installed versions of Vapor. It does not return /usr/local/VAPOR-Deps/2019-Aug during development builds.

GetResourcePath() does work on development builds. Otherwise many other features such as rendering maps wouldn't work.

@sgpearse
Copy link
Collaborator Author

sgpearse commented Jul 5, 2022

@StasJ I see it now, thank you.

HDF5_PLUGIN_PATH is now only required on windows dev/installer builds.

@sgpearse
Copy link
Collaborator Author

sgpearse commented Jul 6, 2022

@sgpearse will look into setting the Windows environment variable within the running executable.

@sgpearse
Copy link
Collaborator Author

@StasJ - Your notes should be addressed. All installers are working except for Windows, #3178. osgl seems to be the problem when we build with the gui. The broken windows build is reported #3178.

@sgpearse sgpearse requested a review from StasJ August 12, 2022 23:12
Copy link
Collaborator

@StasJ StasJ left a comment

Choose a reason for hiding this comment

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

@StasJ - Your notes should be addressed. All installers are working except for Windows, #3178. osgl seems to be the problem when we build with the gui. The broken windows build is reported #3178.

Thanks, looks great. There was one comment you didn't respond to:

This change requires lib-common to link against HDF5. It may be more appropriate for the VDC library to handle this since it handles the use of HDF5 files.

@sgpearse
Copy link
Collaborator Author

@StasJ - Ah, I did miss that. I thought lib common made sense because it handles our resource paths, but no biggie. Fixed.

@sgpearse sgpearse requested a review from StasJ August 16, 2022 15:02
@sgpearse
Copy link
Collaborator Author

Fixes #3179.

@sgpearse
Copy link
Collaborator Author

Fixes #3158.

@sgpearse
Copy link
Collaborator Author

Fixes #3174

@clyne
Copy link
Collaborator

clyne commented Aug 16, 2022

Ready for @StasJ to review again

@clyne
Copy link
Collaborator

clyne commented Aug 17, 2022

Fixes #2816

@clyne
Copy link
Collaborator

clyne commented Aug 17, 2022

@sgpearse this issue has been closed. Presumably everyone should update their own development libraries to be in sync with the DevOps platforms, correct? Where are these libraries available?

thanks!

cc: @NihanthCW @shaomeng @StasJ

@sgpearse
Copy link
Collaborator Author

Presumably everyone should update their own development libraries to be in sync with the DevOps platforms, correct?

Yes.

Where are these libraries available?

Check out the PR description, which has links to bundles for each platform.

@clyne
Copy link
Collaborator

clyne commented Aug 17, 2022

When I click on the link for Mac OSX I get:

Sorry, the file you have requested does not exist.

@sgpearse
Copy link
Collaborator Author

@clyne Broken link is updated on OSX here. PS - I've double checked that the links on CircleCI are targeting the correct libraries and not dead links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants