Missing output directory, breaks the run

[hlokavarapu]

In output.cpp, there is a system call to mkdir if the output path doesn't exist in the constructor of OutputStructure. As opposed to creating the specified directory, the run breaks after HDF5 tries to create a file in a missing directory.

[TedStudley]

This issue is made more complicated by the fact that the output directory is defined in the parameters file instead of hard-coded. Since we can't guarantee that output will be written to output/X, I'm not sure it would be a good idea to change this functionality (or lack of functionality).

It would be possible to fix this by recursively creating any directories on the path that don't exist, but I think that has the possibility to introduce very unexpected behavior if, for example, the output path is set to something like /hime/ted/mc-mini/output/X (typo definitely intended). Adding a better error message than the mess HDF5 comes back with after failing is definitely a good idea, but I'm against automatically creating anything but the top-most directory.

[hlokavarapu]

How about if the command was set to something like, mkdir $PWD/output/exampleOutput?

[TedStudley]

We can't guarantee that that's actually the output directory. Check out the outputPath parameter in the outputParams param file subsection. If you change that to a different folder than the default, then creating an output folder in the current working directory not only won't solve the problem, but will also create an unwanted and potentially unexpected folder, as well.

It would probably be a good idea to either remove the output folder from .gitignore (and re-add output/*) or change the makefile to automatically generate the folder when building the project. Anything but that could cause unexpected behavior.

[hlokavarapu]

I am not sure that I understand your concerns...

If per say /hime/ted/mc-mini/ouput/X is specified, nothing will happen; unless mc-mini is being ran as super user. And that would be silly. Can;t really think of a reason one would do something like that.

But say that I create the folder in $PWD/ouput/. Even if a string of nonsensical folder names is set, it would be under the output folder. I am not sure what kind of unexpected behavior you have in mind here. If you could clarify, that would be great. I am actually curious as to how one could break that.

But more to the point, there was a mkdir command in the outputStructure constructor. I assume the purpose was for missing outputFilePath. I might be mistaken though.

[TedStudley]

I gave a very extreme case, but the underlying problem is still the same. Let's look at two different issues:

I want to have mc-mini output to ~/benchmarks/mc-mini/tau_benchmark, but accidentally set outputFilePath as ~/benvhmarks/mc-mini/tau_benchmark. where 'X' is the backtick (github markdown won't let me escape it properly.) If we recursively create the folders, then we'll end up creating an entire directory tree that we never actually wanted. Furthermore, actually recursively creating the tree adds an unnecessary level of complexity to the output routines. What if one of the directories named already exists as a file, or has the wrong permissions, or has an invalid name? Should we try to handle the error in that case, or just give up? Why do any differently if the expected folders don't exist?

Next, let's say that I want to have mc-mini output to ~/benchmarks/mc-mini/tau_benchmark, but we create the folder in $PWD/output/. Not only does that completely prevent us from creating the folder in the home directory, it also means that where the output ends up depends upon where we call mc-mini from. If I have the following directory structure:

  home/
    mc-mini/
        ./mc-mini
        params

running mc-mini from home with the command mc-mini/mc-mini mc-mini/params would end up giving me

  home/
    mc-mini/
      ./mc-mini
      params
    output/
      benchmarks/
        mc-mini/
          tau_benchmark/
            ...

while running it from inside the mc-mini directory with the command ./mc-mini params would end up giving me

  home/
    mc-mini/
      ./mc-mini
      params
      output/
        benchmarks/
          mc-mini/
            tau_benchmark/
              ...

Neither case will end up giving me the behavior that I want, and even worse, the behavior is different depending on where I run the program from.

Think about what happens when you use ls or any number of other unix command-line tools. No matter where you use ls from, supplying any path to the same folder will always have the same behavior. Similarly, if you try to use something like mkdir to create a directory inside of another non-existent directory, it'll refuse. It won't create a chain of directories that you'll then have to clean up afterwards. Not having the directory auto-create itself isn't even that much of an inconvenience, since it isn't like it has to go through an entire run before you'll discover the problem. You'll get an error immediately after the first timestep. A better solution would be to ensure that we get the error before the first timestep, but that's not really relevant to this particular issue.

[hlokavarapu]

Fair enough. I only want to point out that Aspect does create an ouput directory if it doesn't exist. I do not know how they have handled the above cases, or if they simply ignored them. But when I do find out, I will let you know.

[TedStudley]

Looks like Aspect has exactly the same behavior that I've implemented in mc-mini, albeit with a better error message.

[hlokavarapu]

So both in Aspect and Mc-mini, the output direcory is to be created if it doesn't exist. Except that in mc-mini, that's not the result after run time. An error is thrown and the directory is never created. Eventhough, the code says otherwise:

sprintf (s, "test -e %s", outputPath.c_str()); if (system (s) == 1) { sprintf (s, "mkdir %s", outputPath.c_str()); if (system (s) == -1) { cout << "<Error: couldn't create directory " << outputPath << ">" << endl; exit (-1);; } }

Stepping though the debugger, the problem is the test condition. The result of system("test -e %s") is not 1 if the ouputpath doesn't exist. It is 256 on my computer. This is a bug that should be fixed.

[TedStudley]

That's odd, it's 1 on my machine... Seems as though that's not standardized behavior, though. The IEEE POSIX standard for test -e says:

-e pathname True if pathname resolves to an existing directory entry. False if pathname cannot be resolved.

It seems as though the problem arises from the posix standard for false, which only says:

The false utility shall return with a non-zero exit code.

On my machine, false is implemented as returning 1. This explains why this bug is only happening on your machine!

I'll put in a patch which should solve this (and also squash a potential bug in the call to mkdir.) Isn't portability fun?

[TedStudley]

Quick fix pushed to master in 9fca5b3.