search

ComputationalRadiationPhysics / libSplash

libSplash - Simple Parallel file output Library for Accumulating Simulation data using Hdf5
GNU Lesser General Public License v3.0
15 stars 15 forks source link

Remove Dimensions::getDims #239

Open Flamefire opened 8 years ago

Flamefire commented 8 years ago

This function (https://github.com/ComputationalRadiationPhysics/libSplash/blob/master/src/include/splash/Dimensions.hpp#L252-L263) is wrong, as it does neither honor zero sizes (all dims=0) nor does it work for offsets. As it can't be fixed properly in that design, it should be removed.

ax3l commented 8 years ago

careful, zero-components have still "size==1", e.g., a 2D data set has (X, Y, 1) size and (OX, OY, 0) offset

Flamefire commented 8 years ago

So a 2D zero size is (0, 0, 1)? This looks rather strange... Is this a HDF5/Splash related requirement?

ax3l commented 8 years ago

no there is no zero size in libSplash, the smallest element one can write is of size (1, 1, 1) while its dimensionality (1D, 2D, 3D) is set via dim.

Flamefire commented 8 years ago

Really? What about e.g. writing particles when there are no particles for the current process? Or field slices where the current process has no data because it is outside the slice?

ax3l commented 8 years ago

The limitation only applies to "global" sizes of data sets. Please check https://github.com/ComputationalRadiationPhysics/picongpu/tree/dev/src/picongpu/include/plugins/hdf5 for details.

ax3l commented 8 years ago

zero-sized would be (0, 1, 1) ... don't judge. it's all from having a fixed 3D Dimensions object in splash instead of using a class that can do ND...

Flamefire commented 8 years ago

Well I did not know, that you can't write a zero-sized record and just did it. It actually writes a single value, but doesn't run into any problems besides that.

ax3l commented 8 years ago

It actually writes a single value, ...

yes, sounds like it.

ax3l commented 7 years ago

can this issue be closed or does it need further evaluation?

Flamefire commented 7 years ago

It is not solved as that function is still wrong for the reasons described. Another simple example: 3D size of 1: (1, 1, 1) for which getDims would return 1 which is obviously wrong.

ax3l commented 7 years ago

well it's not completely wrong, just super inconsistent. (1, 1, 1) is indeed one element, but I see your point. needs a general redesign.

Flamefire commented 7 years ago

If you create a 3D object and it says it is 1D it is not inconsistent, it is plain wrong. Or would you use "inconsistent" for "works sometimes but fails silently otherwise"? So remove it for now (see initial description). Better have no result than a wrong one.