Modify

Opened 7 years ago

Last modified 6 years ago

#669 reopened task

Rewrite users of deprecated HDF5 C++ API

Reported by: Erik Schnetter Owned by:
Priority: minor Milestone:
Component: Other Version:
Keywords: Cc:

Description

The C++ API for HDF5 is decprecated. We should examine which code uses it, and rewrite it to use the C API instead. This would allow us to use more system-provided HDF5 installations.

Attachments (0)

Change History (8)

comment:1 Changed 7 years ago by Barry Wardell

One way to achieve this would be to use a basic C++ wrapper around the C API. For this, something along the lines of h5wrapper.cc and h5wrapper.h might be useful.

comment:2 Changed 7 years ago by Frank Löffler

Let's first see which code actually uses it. The prominent example used to be the Visit plugin for Carpet HDF5, but that got rewritten to use the C interface. Is there anything else using the C++ interface? If not we could drop it IMHO.

comment:3 Changed 7 years ago by Erik Schnetter

That would be a good question for a mailing list.

comment:4 Changed 6 years ago by Frank Löffler

Status: newreview

Nothing within the toolkit uses it. It builds without. And the Carpet plugin is better build against the headers within visit anyway because of version problems you typically get otherwise. I suggest to drop this ticket and consider the C++ interface to be not provided by the HDF5 thorn (which in fact it isn't necessarily). Thus, I propose to change the default for HDF5_ENABLE_CXX to 'no'.

comment:5 Changed 6 years ago by Roland Haas

I support the notion of disabling the C++ API in ExternalThorns/HDF5 by default.

comment:6 Changed 6 years ago by Ian Hinder

Status: reviewreopened

I agree. Let's disable the use of the C++ API by default.

Looking at HDF5/configure.sh, it looks like we should just change

   if [ "${HDF5_ENABLE_CXX:=yes}" = 'yes' ]; then

to

  if [ "${HDF5_ENABLE_CXX:=no}" = 'yes' ]; then

However, the script looks like it might not be quite right, as later on this variable is used without a default being set. Additionally, the script does not treat "yes/no" in a caseless way like the other configuration scripts do.

The options supported by a configuration script are listed in configuration.ccl, but their default values are not - this is left up to the configuration script, so every configuration script has to contain programmatic code to manage defaults. I have amended #1042 to address this.

Changing status of this ticket from Review to "reopened" as there is no patch as yet.

comment:7 Changed 6 years ago by Roland Haas

':=' actually assigns default values (see the bash man page). So as long as the line Ian quoted is the first one where the variable appears everything after that has well defined default values. Bash's syntax is definitely rather strange. A less below-the-radar way of setting defaults would be something like:

: ${HDF5_ENABLE_CXX:=yes}

(note the ':' at the beginning of the line).

comment:8 Changed 6 years ago by Erik Schnetter

Here is an even less below-the-radar way:

# Assign default value if unset
: ${HDF5_ENABLE_CXX:=yes}

Note the English text following the "#" character.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as reopened The ticket will remain with no owner.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from (none) to the specified user.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.