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

Make filename creation consistent #234

Closed Flamefire closed 8 years ago

Flamefire commented 8 years ago

Allows passing full name to SerialDataCollector

Previously the SerialDataCollector created names like "foo.h5_0_0_0.h5" , this is now kept as "foo.h5"

For this it also changes the HandleMgr and extends its filename scheme by FNS_FULLNAME where the name is kept unchanged. Additionally checks where added for catching erroneously passing a full name into the DataCollectors when a base name was expected.

@ax3l

ax3l commented 8 years ago

Thank you and I applaud you for directly adding tests! :sparkles:

Can you try to avoid using *nix-only includes? Not sure if unlink-ing the old files is necessary, this can be done in the run scripts.

ax3l commented 8 years ago

I like the changes.

Please make sure that SerialDataCollector is really used with mpiSize of one rank in case of FullFileName without the MPI position appendix, otherwise things will go south.

Flamefire commented 8 years ago

About Unix-Functions: stat is already used in libSplash. And I need to assert that this file does not exist to reliably check for creation. Doing that in the shell script would require deleting all *.h5 files or list them all there which I want to avoid as it is cumbersome.

Checking in SerialDataCollector is not possible as there is no MPI information inside it. It is a SERIALDataCollector in the end. Only information there is attr.mpiPosition which we could assert to be 0. We can also pass this (+1) as the mpiSize into HandleMgr::open and have it checked there. Also it is a bad idea, because the user may have chosen to name the file manually unique.

Edit: Just tried some refactoring to better unify the handling of the filename by moving it into HandleMgr but the design is seriously broken by scattering and mixing different data all over the classes and calls (e.g. different usage of FileCreationAttr) which makes this almost impossible. IMO the library needs a major redesign... Bottom line: It does not get much better.

Flamefire commented 8 years ago

Offline discussion with @ax3l : SerialDC must also check that it is not used with full names, unless it is read-only. This is implemented in f36f925

ax3l commented 8 years ago

SerialDC must also check that it is not used with full names, unless it is read-only

... or mpiSize is ==1

Flamefire commented 8 years ago

@ax3l Can this be merged?

ax3l commented 8 years ago

thank you very much for the work! :sparkles:

ax3l commented 8 years ago

PR is gone to master. reverted!