Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve folder structure of hdf5 / pouts / .dat #166

Merged
merged 8 commits into from
Mar 26, 2021

Conversation

TaigoFr
Copy link
Member

@TaigoFr TaigoFr commented Mar 13, 2021

This PR improves the organization of the folder structure of all the output to the filesystem in several ways.

  • often people have to specify the full path of checkpoint files and plotfiles and restart files in the respective parameters repeateadly (e.g. chk_prefix = "/scratch/mypath/BinaryBH_" and similarly for the other parameters). Now these can be left with just the prefix (i.e. chk_prefix = BinaryBH_) and one new parameter was introduced: output_path , the base path prepending all outputs. It defaults to "", so it doesn't break any old code. If non-empty, output_path must exist already before the start of the job.
  • hdf5 files, pout files and .dat files (puncture files and extraction files) can now be put in their own specific folder using hdf5_subpath, pout_subpath and data_subpath (output_path prepends all of them). Again, they default to "", so they don't break any code. Unlike output_path, any subpath doesn't need to be created before hand, this PR takes care of creating them (even if they contain multiple levels of uncreated directories which need to be created recursively).
  • the name of the pout files can also be changed now to whatever prefix one wants, using the parameter pout_prefix. This might be useful not to override old pout files.
  • Extraction data typically has a lot of files when write_extraction is on. These files can be included in another directory by specifying extraction_subpath (which defaults to data_subpath)
  • pout verbosity can be reduced with print_progress_only_to_rank_0, which will print the number of boxes per rank on ranks other than rank 0 only at regrid time steps. For large number of nodes this improves performance.

Example:
Adding to the BinaryBH Example params_very_cheap.txt the following:

output_path = "results" # must exist!
hdf5_subpath = "hdf5"
pout_subpath = "pout"
data_subpath = "data"
pout_prefix = "poutChanged"

would put all the files in a folder results/ with files in subdirectories: hdf5's in results/hdf5, pout's in results/pout and .dats files in results/data. It would also change the name of the pout files to poutChanged.

Let me know if you think there is a better naming for any parameter.

@TaigoFr TaigoFr requested review from KAClough and mirenradia March 13, 2021 00:55
@TaigoFr TaigoFr changed the title Improve folder structure of hdf5 / pouts / .dat Improve folder structure of hdf5 / pouts / .dat Mar 13, 2021
Fully backwards compatible.
The base folder where all files are outputted is now 'output_path', defaulted to "".
This folder must exist.

HDF5's are written to 'output_path + hdf5_subpath'.
pout files are written to 'output_path + pout_subpath' and can be renamed to 'pout_prefix'.
.dat files (punctures, extraction files) are written to 'output_path + extraction_subpath'.
All subpaths are defaulted to "". They are created at runtime, so need not exist before.
@TaigoFr TaigoFr force-pushed the enhancement/improve_folder_structure branch from 1b7eb97 to dbc0fce Compare March 13, 2021 01:05
Copy link
Member

@mirenradia mirenradia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look great, Tiago! Thanks for doing this 😄 I have tested that it works for me. On my laptop it wasn't necessary for the output_path to exist before running but perhaps this is architecture/compiler/OS dependent.

I have some [relatively minor] comments.

Source/utils/GRParmParse.hpp Outdated Show resolved Hide resolved
Source/utils/GRParmParse.hpp Outdated Show resolved Hide resolved
Source/utils/GRParmParse.hpp Outdated Show resolved Hide resolved
Comment on lines 413 to 417
GRParmParse::folder_exists(output_path),
"should be a valid folder");
check_parameter("pout_path", pout_path,
GRParmParse::folder_exists(pout_path),
"should be a valid folder");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some respects, this check is pointless since either the path will have been created by the system (as on my laptop) or the code will have failed because the path doesn't exist and it tried to write some default parameter messages to the pout file. Maybe this check needs to be written to std::cout instead in read_filesystem_params and not use the usual check_parameter functions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the folder was created on your laptop because even the mkdir_recursive for pout's created it, since it's recursive!!! But had you not used a pout/hdf5/.dat subpath, then it would not have been created. So what we mean by "must exist" is probably that we do not take direct responsibility in creating it ourselves.
I will remove the pout check -> indeed we create it ourselves, so it will exist if nothing goes wrong. The output_path check will give an error if the directory doesn't exist and you didn't specify any pout subpath (and hence output_path was not created by mkdir_recursive)

Source/utils/GRParmParse.hpp Outdated Show resolved Hide resolved
Source/GRChomboCore/ChomboParameters.hpp Outdated Show resolved Hide resolved
Source/GRChomboCore/ChomboParameters.hpp Outdated Show resolved Hide resolved
Source/AMRInterpolator/SurfaceExtraction.impl.hpp Outdated Show resolved Hide resolved
Source/BlackHoles/PunctureTracker.cpp Outdated Show resolved Hide resolved
Source/utils/GRParmParse.hpp Outdated Show resolved Hide resolved
@mirenradia
Copy link
Member

I've just thought it might be useful to put the mode integral files into a subdirectory of the extraction_subpath directory? When write_extraction == true, there are a lot of Weyl4ExtractionOut_ files and it would be nice to separate them. On the other hand, I think most users don't set write_extraction = true and the default is false.

@TaigoFr TaigoFr force-pushed the enhancement/improve_folder_structure branch from 974b089 to bcf138a Compare March 17, 2021 22:12
To print the number of boxes on all ranks on regrids, this is done in postRegrid.
postRegrid is not called at t=0 (or at a restart), so GRAMRLevel::advance prints if it's rank 0 OR if it's t=restart_time
@TaigoFr TaigoFr force-pushed the enhancement/improve_folder_structure branch from bcf138a to f772ac2 Compare March 17, 2021 22:17
@mirenradia mirenradia added the enhancement Modification of existing feature/general improvement label Mar 17, 2021
TaigoFr added 3 commits March 18, 2021 11:42
Also fixed bug (constraint norms should be sent to appropriate directory at 'data_path')
@mirenradia mirenradia self-requested a review March 20, 2021 16:12
Copy link
Member

@mirenradia mirenradia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ❤ the nicely formatted parameter files. Unfortunately, I still found a few comments to make.

Examples/KerrBH/params.txt Outdated Show resolved Hide resolved
Examples/KerrBH/params_cheap.txt Outdated Show resolved Hide resolved
Examples/ScalarField/params.txt Outdated Show resolved Hide resolved
Source/GRChomboCore/ChomboParameters.hpp Outdated Show resolved Hide resolved
Source/GRChomboCore/SimulationParametersBase.hpp Outdated Show resolved Hide resolved
Source/GRChomboCore/SimulationParametersBase.hpp Outdated Show resolved Hide resolved
Source/utils/FilesystemTools.hpp Outdated Show resolved Hide resolved
Source/AMRInterpolator/SurfaceExtraction.impl.hpp Outdated Show resolved Hide resolved
@mirenradia
Copy link
Member

I have tested all the parameter files with the -check_params flag and there were no issues.

@mirenradia mirenradia self-requested a review March 22, 2021 22:54
Copy link
Member

@mirenradia mirenradia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy for this to be merged.

@mirenradia mirenradia merged commit a8135ce into main Mar 26, 2021
@mirenradia mirenradia deleted the enhancement/improve_folder_structure branch March 26, 2021 11:22
mirenradia added a commit to mirenradia/GRChombo-public that referenced this pull request Feb 3, 2023
…prove_folder_structure

Improve folder structure of hdf5 / pouts / .dat
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Modification of existing feature/general improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants