Conversation
| hpxml_path = args[:hpxml_path] | ||
| unless (Pathname.new hpxml_path).absolute? | ||
| hpxml_path = File.expand_path(hpxml_path) | ||
| hpxml_path = File.join(runner.workflow.absoluteRootDir.to_s, hpxml_path) |
shorowit
left a comment
shorowit
left a comment
There was a problem hiding this comment.
Thanks, @rajeee. I'm glad to see the CI ran successfully but this makes me a bit nervous. For example, I know that PAT users had issues in the past. We'll need to check that it still works for that use case.
Is this meant to fix #1956?
| # @param args [Hash] Map of :argument_name => value | ||
| # @return [nil] | ||
| def set_file_paths(args) | ||
| def set_file_paths(runner, args) |
There was a problem hiding this comment.
Add to method documentation above
| "hpxml_path": "sample_files/base.xml", | ||
| "output_dir": "run", |
There was a problem hiding this comment.
I completely agree that the hpxml_path makes much more sense like this.
Can output_dir be anything or does it have to be "run"?
There was a problem hiding this comment.
It has to match the "run_directory" argument above (in line 2). If run_directory is not specified, it defaults to "run" and output_dir has to be "run" in that case. I think we should just remove this argument from the measure. We can find out the run_directory using runner.workflow.absoluteRunDir.to_s
There was a problem hiding this comment.
think we should just remove this argument from the measure.
We can't remove it, it is one of the most commonly used arguments (e.g. run_simulation.rb -x foo.xml -o my_output_dir).
There was a problem hiding this comment.
We can still allow run_simulation.rb to have output dir. Looks like the current behavior is that it creates a my_output_dir/run folder and places the results there. We can replicate this without needing the output_dir parameter for the measure.
Co-authored-by: Scott Horowitz <scott.horowitz@nrel.gov>
It would be helpful to get @joseph-robertson feedback on whether it causes any problem for PAT.
Maybe - if we can get it all sorted out! |
2 participants
Pull Request Description
Currently, relative paths in the measure arguments are resolved relative to the current working directory. This is problematic because:
When the measures are run directly from the workflow file (using
openstudio run -w workflow_file.osw), the current directory is something like './run/000_measure_class_name/'. So, in order to put the results into the run folder, one has to use..or absolute path. To refer to files at osw level, one has to use../../. This is confusing and not intuitive.When the measure is run through apply_measure, which is what the run_simulation.rb in OS-HPXML, and many meta measures and tests do, the current directory resolves to where the test/script is invoked from. This causes the relative path to resolve to different paths depending upon how the measure is called (via openstudio command or via the apply_measure). This makes it impossible to write osw files that can be run from both the command line using openstudio command or run through apply_measure by a test unless all paths are absolute.
This PR resolves the relative paths in the OSW relative to the OSW file itself, which alleviates this problem.
Checklist
Not all may apply:
EPvalidator.xml) has been updatedopenstudio tasks.rb update_hpxmls)HPXMLtoOpenStudio/tests/test*.rband/orworkflow/tests/test*.rb)openstudio tasks.rb update_measureshas been run