HDF5 logger does not check data type correctly

[Charlottez112]

Description

Currently hoomd.write.HDF5Log checks the data type of the values with dtype = "f8" if isinstance(value, float) else "i8", which does not account for data types such as numpy.float64. We should either include these types because users might define custom quantities which are calculated from NumPy etc, or provide an option for the user to specify the data type? In the following example, originally the acceptance ratio is an integer, but after several trial moves are made it will be float. We will probably also need to account for this when checking the data types, or document this behavior.

Script

@log(category="scalar", requires_run=True)
def acceptance_ratio(self):
    total_moves = sum(self._counter)
    accept_moves = self._counter[0]
    if total_moves == 0:
        return 0
    else:
        return accept_moves / total_moves

# Create a logger to log the acceptance ratio
def create_writer(self, log_fn, mode, dump_period):
    logger = hoomd.logging.Logger(categories=["scalar"])
    logger.add(self, quantities=[
                    "acceptance_ratio",
                ])
     return hoomd.write.HDF5Log(
            trigger=hoomd.trigger.Periodic(dump_period, 1),
            filename=log_fn,
            logger=logger,
            mode=mode
        )

Input files

No response

Output

[0, 0, ...., 1, 1, ..., 1]

Expected output

No response

Platform

Linux

Installation method

glotzerlab-software container

HOOMD-blue version

4.1.0

Python version

3.11.3

[joaander]

You can work around this for now with:

    if total_moves == 0:
        return float(0)
    else:
        return float(accept_moves / total_moves)

We should document the behavior better and show examples with explicit type casting in custom logged quantities. I don't see how it is possible for HOOMD to predict what future data types your custom log routine might return.

It not a performance penalty, we could consider adding warnings when the data type changes from one frame to the next.

@b-butler how can we generically handle coercing any "floating point" type into "f8"?
https://github.com/glotzerlab/hoomd-blue/blob/df6075d9a4c180e16ea9ccd68bb62116ea5a3fed/hoomd/write/hdf5.py#L212C45-L212C45

[Charlottez112]

We should document the behavior better and show examples with explicit type casting in custom logged quantities. I don't see how it is possible for HOOMD to predict what future data types your custom log routine might return.

I was asking Brandon about this. Right now it's almost like the default type is i8, and it can be problematic when the user does something like my minimal example, where 0 is logged instead of a positive number. But defaulting it to 'f8', at least we won't have that problem. What about providing an option to the user? Although I'm not sure what the best practice for using h5py is

[joaander]

Yes, but under what somewhat complete set of conditions should HDF5Log store integer types? So that another user who provides a numpy.int64 (or some other integer type) does not run into problems where there is a loss of precision during the conversion to float?