Respect WorkflowJSON file paths over default ones

[macumber]

Issue overview

The OpenStudio Workflow Gem attempts to put the ./generated_files dir as the first path in the WorkflowJSON file
https://github.com/NREL/OpenStudio-workflow-gem/blob/develop/lib/openstudio/workflow/jobs/run_initialization.rb#L91

However, subsequent calls to WorkflowJSON::filePaths insert the default paths (e.g. ./files, ./weather) before the paths in the WorkflowJSON file.
https://github.com/NREL/OpenStudio/blob/develop/src/utilities/filetypes/WorkflowJSON.cpp#L424

WorkflowJSON_Impl::filePaths should insert default paths after paths in the JSON file.

Current Behavior

Files generated during the run go into a ./files directory if it exists instead of the ./generated_files dir

Expected Behavior

Files generated during the run go into a ./generated_files directory

Steps to Reproduce

1. Run a workflow that generates a file, file will go in ./generated_files
2. Create a ./files directory in the WorkflowJSON root dir
3. Re-run a workflow that generates a file, file will go in ./files

Context

Makes it difficult to tell users where to find files generated during the run

[tijcolem]

@macumber Thanks for reporting this. This is most likely in the workflow gem and perhaps should be transfered there.

[macumber]

I think the issue is in workflowjson::filePaths in this repo, that is what puts the default paths ahead of user ones

[macumber]

Specifically I think the fix is to change:

  std::vector<openstudio::path> WorkflowJSON_Impl::filePaths() const {
    std::vector<openstudio::path> result;

    Json::Value defaultValue(Json::arrayValue);

    Json::Value paths = m_value.get("file_paths", defaultValue);
    paths.append("./files");
    paths.append("./weather");
    paths.append("../../files");
    paths.append("../../weather");
    paths.append("./");

    Json::ArrayIndex n = paths.size();
    for (Json::ArrayIndex i = 0; i < n; ++i) {
      result.push_back(toPath(paths[i].asString()));
    }

    return result;
  }

to

  std::vector<openstudio::path> WorkflowJSON_Impl::filePaths() const {
    std::vector<openstudio::path> result;

    Json::Value defaultValue(Json::arrayValue);

    Json::Value paths = m_value.get("file_paths", defaultValue);

    Json::ArrayIndex n = paths.size();
    for (Json::ArrayIndex i = 0; i < n; ++i) {
      result.push_back(toPath(paths[i].asString()));
    }

    paths.append("./files");
    paths.append("./weather");
    paths.append("../../files");
    paths.append("../../weather");
    paths.append("./");

    return result;
  }

[tijcolem]

@macumber Sorry for delay on this. Picking up back logged issues. I jumped back into this and tested the proposed code change that you provided (thanks btw). The actual order in the return value std::vector<openstudio::path> result is the same for both code snippets above where the generated_files path value is at index 0

printing out the contents of std::vectoropenstudio::path result after populated.

std::cout <<  "returned results" << std::endl;
  for (auto i: result) {
  std::cout <<  i << std::endl;
 }

output

/Users/tcoleman/git/OpenStudio-workflow-gem/spec/files/compact_osw/generated_files
./files
./weather
../../files
../../weather
./
./files
./weather
../../files
../../weather
./

Received same output for both.

I did notice duplicate default path values (e.g. ./files and ./weather) that are set in the OpenStudio-workflow-gem here https://github.com/NREL/OpenStudio-workflow-gem/blob/develop/lib/openstudio/workflow_json.rb#L324-L328. I don't think this is causing the issue you're having but should be corrected once this bug is identified. I'm thinking maybe this is somewhere else in the code as the path order in result seems to be fine.

[tijcolem]

@macumber If you still have the offending workflow that produces content in the generated_files could you attach it or send to me email?

[macumber]

Thanks for looking Tim. I thought there was a component of openstudiocoalition/OpenStudioApplication#280 that was due to the OpenStudio SDK. However, looking at it again, I think the code here is correct. I could not reproduce any issues.