libSplash icon indicating copy to clipboard operation
libSplash copied to clipboard

Remove Dimensions::getDims

Open Flamefire opened this issue 9 years ago • 12 comments

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.

Flamefire avatar Jun 17 '16 08:06 Flamefire

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

ax3l avatar Jun 17 '16 11:06 ax3l

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

Flamefire avatar Jun 17 '16 11:06 Flamefire

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.

ax3l avatar Jun 17 '16 12:06 ax3l

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?

Flamefire avatar Jun 17 '16 12:06 Flamefire

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 avatar Jun 17 '16 12:06 ax3l

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...

ax3l avatar Jun 17 '16 12:06 ax3l

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.

Flamefire avatar Jun 17 '16 12:06 Flamefire

It actually writes a single value, ...

yes, sounds like it.

ax3l avatar Jun 17 '16 12:06 ax3l

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

ax3l avatar Oct 26 '16 09:10 ax3l

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.

Flamefire avatar Oct 26 '16 09:10 Flamefire

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.

ax3l avatar Oct 26 '16 11:10 ax3l

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.

Flamefire avatar Oct 26 '16 11:10 Flamefire