Add cuba_keys to add_dataset method of HDF5 CUDS

[roigcarlo]

The add_dataset method of the hdf5 file wrapper (H5CUDS) should provide an optional cuba_keys argument to use for the supported cuba keywords that are needed to be copied to the file.

https://publicwiki-01.fraunhofer.de/SimPhoNy-Project/index.php/Supported_CUBA_keys_for_container .

[tuopuu]

In the current implementation of the H5CUDS backend, tables with 66 columns (corresponding to the current total of the CUBA keys) are used to store CUDS data regardless of the number of different CUBA keys defined for the saved objects.

I understand that for solving this issue #229 we do not need to modify the backend, but instead just modify the add_particles, add_lattice and add_mesh so that unwanted CUBA keys are not stored in file (while there still exist all those 66 columns). The code itself could be something along the lines of

# from h5_cuds.add_particles
...
if cuba_keys is not None:
        for p in particles.iter_particles():
                new_p = Particle.from_particle(p)
                new_p.data = new_p.data(cuba_keys[CUDSItem.PARTICLE])
                h5_particles.add_particles(new_p)
        ...
else:
            h5_particles.add_particles(iter_particles())
            h5_particles.add_bonds(particles.iter_bonds())

Unfortunately, this might turn out to be highly inefficient. While there is currently no way of iterating over CUDSItems with masking a part of their CUBA keys, I don't see a better way to fix this issue #229. Any comments, @nathanfranklin, @roigcarlo, etc?

Anyway, a separate issue should be made for changing the file backend where not all columns are allocated.

[nathanfranklin]

Unfortunately, this might turn out to be highly inefficient. While there is currently no way of iterating over CUDSItems with masking a part of their CUBA keys, I don't see a better way to fix this issue

You mean, something like the following, right? (I didn't follow the new_p.data = new_p.data(cuba_keys[CUDSItem.PARTICLE]) line).

if cuba_keys is not None:
        for p in particles.iter_particles():
                p.data = DataContainer({key:data[key] for key in data if key in cuba_keys[CUDSItem.PARTICLE]})
                h5_particles.add_particles([p])

Yes, this seems to work. Alternatively (as a future improvement), one could pass this cuba_keys information on to H5Particles's add_particles (or a future 'add_particles_filter' method).

I haven't thought about it much but my initial gut feeling is to just do what you proposed and make efficiencies improvements down the line.

Anyway, a separate issue should be made for changing the file backend where not all columns are allocated.

Yeah, if this is a concern then a separate issue should be opened and discussed. Especially, I think it should be discussed more as I feel it is important to ensure that we can add dataset with only 2 attribute (e.g. cuba_key has CUBA.MASS, CUBA.RADIUS) but still be able to update entities of that data set (in the file) so that a 3rd attribute can be added (i.e. CUBA.VELOCITY is on the points in addition to CUBA.MASS and CUBA.RADIUS)

[nathanfranklin]

Any comments, @nathanfranklin, @roigcarlo, etc?

I am adding some others ( @itziakos [who originally wrote the related proposal), @mehdisadeghi @ahashibon ) who might want to comment

[tuopuu]

Yes, this seems to work. Alternatively (as a future improvement), one could pass this cuba_keys information on to H5Particles's add_particles (or a future 'add_particles_filter' method).

Yes, my piece of code was not actually working, but yours does, and it was what I was trying to do.

[tuopuu]

Especially, I think it should be discussed more as I feel it is important to ensure that we can add dataset

I agree, this is important. I'm not sure if PyTables table object can currently do it in other way than completely rebuilding of the table, and we should therefore think of other constructs that could replace tables. It's a big problem and we should leave the discussion under a new issue number, or maybe inside the wiki, at first.

[ahashibon]

Agree with @nathanfranklin. Lets first now implement as @tuopuu and @nathanfranklin discussed. (Other improvements can be considered later when really needed)

[itziakos]

Please note that the special keyword argument for the HDF5 storage backend has already been proposed and to a certain extend discussed in the wiki (proposal for Supported CUBA keys for container).

[nathanfranklin]

Please note that the special keyword argument for the HDF5 storage backend has already been proposed and to a certain extend discussed in the wiki (proposal for Supported CUBA keys for container).

I added the link (https://publicwiki-01.fraunhofer.de/SimPhoNy-Project/index.php/Supported_CUBA_keys_for_container ) here and to the first comment. The link wasn't listed when this issue was first created from part of #217. This issue (#229), #223 and #217 are meant to implement the Supported CUBA keys for container proposal.

I think @tuopuu is aware of and working from the wiki proposal and related discussions.

[tuopuu]

I think @tuopuu is aware of and working from the wiki proposal and related discussions.

Yes, I'm aware of it. The implementation will be based on what was discussed there.

[tuopuu]

This issue is covered in #244. Closing.