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

Kimmy/vfd doc #230

Merged
merged 36 commits into from
Oct 27, 2021
Merged

Kimmy/vfd doc #230

merged 36 commits into from
Oct 27, 2021

Conversation

jya-kmu
Copy link
Contributor

@jya-kmu jya-kmu commented Sep 15, 2021

Documentation and config files for hdf5-iotest using Hermes VFD.

@coveralls
Copy link

coveralls commented Sep 15, 2021

Coverage Status

Coverage decreased (-0.04%) to 81.293% when pulling 8acfad6 on kimmy/vfd_doc into e10e815 on master.

@ChristopherHogan
Copy link
Collaborator

Can you remove the .DS_Store files and add them to .gitignore?


## Build Hermes
```bash
git clone https://github.com/jya-kmu/hdf5.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be the hermes url?


## Build hdf5 with Hermes VFD
```bash
git checkout https://github.com/jya-kmu/hdf5.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

checkout -> clone

@ChristopherHogan
Copy link
Collaborator

In general I don't like putting files in source control that can easily be generated, especially a large number of files that are 98% similar. My preference would be to have a driver program (bash, python, cpp, whatever) that generates temporary .conf and .ini files based on command line arguments, runs the benchmark, then removes the files. Usage would be something like this:

run_vfd_bench --dpe round_robin --split yes --hierarchy ram,nvme --mount-points ./,/home/user/nvme ...

The benefits of this approach would be:

  • Keeps the directories in the repo small and clean.
  • Easier to maintain. If we change something about the hermes.conf file, then we have to modify dozens of files. However, if we kept the conf files for the VFD benchmark in a script as a template, we would only have to modify it in one place.
  • Easier to modify. If I want to run these benchmarks I have to replace "kmu" with "chogan" in all the .conf files. I would much prefer passing a single command line argument to temporarily modifying dozens of files. If a username is in source control, it is only useful for one person. If it is customizable, it is useful for everyone.
  • Easier for the PR reviewer. I would much rather review one script with a conf file template and generator than the 100 files in this PR.

I'm not saying you have to do this. It's just something to think about.

@jya-kmu
Copy link
Contributor Author

jya-kmu commented Sep 16, 2021

That makes sense. It is not only for the VFD test.

@ChristopherHogan
Copy link
Collaborator

The approach I was thinking of for a config generator has the advantages of

  1. Allowing any number of devices/targets.
  2. Allowing optional arguments, using a default value for missing config options that aren't specified (you require all options to be specified).
  3. No complex parsing necessary.
  4. No initial file necessary.

Here's an outline.

#include <regex>
#include <string>

// Create a config template with capitalized VARIABLE_NAMES as placeholders for
// config parameter values
std::string conf_template = R"(
num_devices = NUM_DEVICES;
num_targets = NUM_TARGETS;
capacities_mb = CAPACITIES;
block_sizes_kb = BLOCK_SIZES;
num_slabs = NUM_SLABS;
slab_unit_sizes = {
  SLAB_UNIT_SIZES
};
desired_slab_percentages = {
  SLAB_PERCENTAGES
};

etc ...

)";

int main() {

  // Create a default Config
  hermes::Config conf = hermes::InitDefaultConfig();

  // Parse command line options and overwrite default Config values with the
  // user-supplied values.

  // Replace variables in the config template with the values in conf
  conf_template = std::regex_replace(conf_template, std::regex("NUM_DEVICES"),
                                     std::to_string(conf.num_devices));
  conf_template = std::regex_replace(conf_template, std::regex("NUM_TARGETS"),
                                     std::to_string(conf.num_targets));
  // etc.

  // Finally, write the final conf_template string to a file. The filename
  // should be passed via command line option, with ./hermes.conf as the default

  return 0;
}

@jya-kmu jya-kmu merged commit ffe2444 into master Oct 27, 2021
@jya-kmu jya-kmu deleted the kimmy/vfd_doc branch October 27, 2021 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants