Repository: NREL/OpenStudio (language: python)

Issue:
Support relative paths when FTing `Schedule:File` objects
Enhancement Request
--------------
IDF file paths for, e.g., the `Schedule:File` object end up as absolute paths after Forward Translation. Users download e.g. ResStock End Use Load Profile models, try to run them, and encounter errors because the absolute file paths don't exist on their computer.

It'd be nice if there was a way to preserve relative paths in the IDF. (As far as I can tell, the `OS:External:File` object does support relative paths, but they do not get maintained in the final IDF.)

It seems like the implementation could:
1. Add a FT option to use relative paths, or
2. Preserve the `OS:External:File` path in the IDF `Schedule:File` (which allows the user to choose if they want to use relative or absolute paths), or
3. ???

I'm not sure if the request applies to other objects.

cc @joseph-robertson 
[internal] Schedule:File relative paths mod
Pull request overview
---------------------

- Modify #4879
- Rename IDD Field
- Move path logic into model `openstudio::path ScheduleFile::translatedFilePath() const`

I'm building these changes, will hopefully report back soonish


Discussion / hints:

Fix patch:
diff --git a/resources/model/OpenStudio.idd b/resources/model/OpenStudio.idd
index 928bc8b9665..2541bf85f55 100644
--- a/resources/model/OpenStudio.idd
+++ b/resources/model/OpenStudio.idd
@@ -5104,13 +5104,20 @@ OS:Schedule:File,
        \key 20
        \key 30
        \key 60
-  A7 ; \field Adjust Schedule for Daylight Savings
+  A7,  \field Adjust Schedule for Daylight Savings
        \note "No" means do not include Daylight Savings Time in the schedule, instead, use the schedule directly from the Schedule:File csv (default)
        \note "Yes" means include Daylight Savings Time to the schedule
        \type choice
        \key Yes
        \key No
        \default Yes
+  A8;  \field Translate File With Relative Path
+       \note "No" means the absolute path of the ExternalFile is resolved and translated to IDF.
+       \note "Yes" means only the filename (relative) is translated
+       \type choice
+       \default No
+       \key Yes
+       \key No
 
 \group OpenStudio Geometry
 
diff --git a/src/energyplus/ForwardTranslator/ForwardTranslateScheduleFile.cpp b/src/energyplus/ForwardTranslator/ForwardTranslateScheduleFile.cpp
index eb4d46bac8a..22b049411b2 100644
--- a/src/energyplus/ForwardTranslator/ForwardTranslateScheduleFile.cpp
+++ b/src/energyplus/ForwardTranslator/ForwardTranslateScheduleFile.cpp
@@ -40,17 +40,8 @@ namespace energyplus {
         idfObject.setString(openstudio::Schedule_FileFields::ScheduleTypeLimitsName, idfScheduleTypeLimits->name().get());
       }
     }
-
-    path filePath = modelObject.externalFile().filePath();
-    if (!exists(filePath)) {
-      LOG(Warn, "Cannot find file \"" << filePath << "\"");
-    } else {
-      // make the path correct for this system
-      filePath = system_complete(filePath);
-    }
-
     // DLM: this path is going to be in the temp dir, might want to fix it up when saving model temp dir
-    idfObject.setString(openstudio::Schedule_FileFields::FileName, toString(filePath));
+    idfObject.setString(openstudio::Schedule_FileFields::FileName, toString(modelObject.translatedFilePath()));
 
     idfObject.setInt(openstudio::Schedule_FileFields::ColumnNumber, modelObject.columnNumber());
     idfObject.setInt(openstudio::Schedule_FileFields::RowstoSkipatTop, modelObject.rowstoSkipatTop());
diff --git a/src/energyplus/Test/ScheduleInterval_GTest.cpp b/src/energyplus/Test/ScheduleInterval_GTest.cpp
index c9f9a24deb8..13ab2cd2af9 100644
--- a/src/energyplus/Test/ScheduleInterval_GTest.cpp
+++ b/src/energyplus/Test/ScheduleInterval_GTest.cpp
@@ -20,6 +20,8 @@
 #include "../../model/ScheduleFixedInterval_Impl.hpp"
 #include "../../model/ScheduleVariableInterval.hpp"
 #include "../../model/ScheduleVariableInterval_Impl.hpp"
+#include "../../model/ExternalFile.hpp"
+#include "../../model/ExternalFile_Impl.hpp"
 #include "../../model/ScheduleFile.hpp"
 #include "../../model/ScheduleFile_Impl.hpp"
 #include "../../model/YearDescription.hpp"
@@ -27,6 +29,10 @@
 
 #include <utilities/idd/IddEnums.hxx>
 
+#include "../../utilities/core/PathHelpers.hpp"
+#include "../../utilities/core/Filesystem.hpp"
+#include <resources.hxx>
+
 #include <boost/regex.hpp>
 
 #include <sstream>
@@ -1850,3 +1856,138 @@ TEST_F(EnergyPlusFixture, DISABLED_ForwardTranslator_ScheduleVariableInterval_Da
   // check that there were XX untils
   //EXPECT_EQ(864, numUntils);
 }
+
+TEST_F(EnergyPlusFixture, ScheduleFileRelativePath) {
+
+  openstudio::path absoluteScheduleFilePath = resourcesPath() / toPath("model/schedulefile.csv");
+  ASSERT_TRUE(openstudio::filesystem::is_regular_file(absoluteScheduleFilePath));
+  ASSERT_TRUE(absoluteScheduleFilePath.is_absolute());
+
+  openstudio::path curDirPath = boost::filesystem::current_path();
+  openstudio::path relativeScheduleFilePath = openstudio::filesystem::relative(absoluteScheduleFilePath, curDirPath);
+  ASSERT_TRUE(relativeScheduleFilePath.is_relative());
+
+  // ScheduleFile(external_file(/abs/path))
+  {
+    Model model;
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ExternalFile>().size());
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ScheduleFile>().size());
+
+    boost::optional<ExternalFile> external_file = ExternalFile::getExternalFile(model, openstudio::toString(absoluteScheduleFilePath));
+    ASSERT_TRUE(external_file);
+    ScheduleFile schedule(*external_file);
+    EXPECT_TRUE(schedule.setTranslateFileWithRelativePath(true));
+    EXPECT_TRUE(schedule.translateFileWithRelativePath());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ScheduleFile>().size());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ExternalFile>().size());
+    ExternalFile externalfile = schedule.externalFile();
+    EXPECT_EQ(1u, externalfile.scheduleFiles().size());
+    EXPECT_EQ(openstudio::toString(absoluteScheduleFilePath.filename()), externalfile.fileName());
+    EXPECT_TRUE(toPath(externalfile.fileName()).is_relative());
+    EXPECT_FALSE(externalfile.filePath().is_relative());
+    EXPECT_NE(toPath(externalfile.fileName()), externalfile.filePath());
+
+    ForwardTranslator ft;
+    Workspace workspace = ft.translateModel(model);
+
+    std::vector<WorkspaceObject> objects = workspace.getObjectsByType(IddObjectType::Schedule_File);
+    ASSERT_EQ(1u, objects.size());
+
+    boost::optional<std::string> fileName = objects[0].getString(2);  // File Name
+    ASSERT_TRUE(fileName);
+    EXPECT_TRUE(toPath(fileName.get()).is_relative());  // rel path
+    EXPECT_NE(externalfile.filePath(), fileName.get());
+  }
+
+  // ScheduleFile(external_file(/rel/path))
+  {
+    Model model;
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ExternalFile>().size());
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ScheduleFile>().size());
+
+    boost::optional<ExternalFile> external_file = ExternalFile::getExternalFile(model, openstudio::toString(relativeScheduleFilePath));
+    ASSERT_TRUE(external_file);
+    ScheduleFile schedule(*external_file);
+    EXPECT_TRUE(schedule.setTranslateFileWithRelativePath(true));
+    EXPECT_TRUE(schedule.translateFileWithRelativePath());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ScheduleFile>().size());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ExternalFile>().size());
+    ExternalFile externalfile = schedule.externalFile();
+    EXPECT_EQ(1u, externalfile.scheduleFiles().size());
+    EXPECT_EQ(openstudio::toString(relativeScheduleFilePath.filename()), externalfile.fileName());
+    EXPECT_TRUE(toPath(externalfile.fileName()).is_relative());
+    EXPECT_FALSE(externalfile.filePath().is_relative());
+    EXPECT_NE(toPath(externalfile.fileName()), externalfile.filePath());
+
+    ForwardTranslator ft;
+    Workspace workspace = ft.translateModel(model);
+
+    std::vector<WorkspaceObject> objects = workspace.getObjectsByType(IddObjectType::Schedule_File);
+    ASSERT_EQ(1u, objects.size());
+
+    boost::optional<std::string> fileName = objects[0].getString(2);  // File Name
+    ASSERT_TRUE(fileName);
+    EXPECT_TRUE(toPath(fileName.get()).is_relative());  // rel path
+    EXPECT_NE(externalfile.filePath(), fileName.get());
+  }
+
+  // ScheduleFile(model, /abs/path)
+  {
+    Model model;
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ExternalFile>().size());
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ScheduleFile>().size());
+
+    ScheduleFile schedule(model, openstudio::toString(absoluteScheduleFilePath));
+    EXPECT_TRUE(schedule.setTranslateFileWithRelativePath(true));
+    EXPECT_TRUE(schedule.translateFileWithRelativePath());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ScheduleFile>().size());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ExternalFile>().size());
+    ExternalFile externalfile = schedule.externalFile();
+    EXPECT_EQ(1u, externalfile.scheduleFiles().size());
+    EXPECT_EQ(openstudio::toString(absoluteScheduleFilePath), externalfile.fileName());
+    EXPECT_FALSE(toPath(externalfile.fileName()).is_relative());
+    EXPECT_FALSE(externalfile.filePath().is_relative());
+    EXPECT_EQ(toPath(externalfile.fileName()), externalfile.filePath());
+
+    ForwardTranslator ft;
+    Workspace workspace = ft.translateModel(model);
+
+    std::vector<WorkspaceObject> objects = workspace.getObjectsByType(IddObjectType::Schedule_File);
+    ASSERT_EQ(1u, objects.size());
+
+    boost::optional<std::string> fileName = objects[0].getString(2);  // File Name
+    ASSERT_TRUE(fileName);
+    EXPECT_FALSE(toPath(fileName.get()).is_relative());  // abs path
+    EXPECT_EQ(externalfile.filePath(), fileName.get());
+  }
+
+  // ScheduleFile(model, /rel/path)
+  {
+    Model model;
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ExternalFile>().size());
+    EXPECT_EQ(0u, model.getConcreteModelObjects<ScheduleFile>().size());
+
+    ScheduleFile schedule(model, openstudio::toString(relativeScheduleFilePath));
+    EXPECT_TRUE(schedule.setTranslateFileWithRelativePath(true));
+    EXPECT_TRUE(schedule.translateFileWithRelativePath());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ScheduleFile>().size());
+    EXPECT_EQ(1u, model.getConcreteModelObjects<ExternalFile>().size());
+    ExternalFile externalfile = schedule.externalFile();
+    EXPECT_EQ(1u, externalfile.scheduleFiles().size());
+    EXPECT_EQ(openstudio::toString(relativeScheduleFilePath), externalfile.fileName());
+    EXPECT_TRUE(toPath(externalfile.fileName()).is_relative());
+    EXPECT_TRUE(externalfile.filePath().is_relative()) << externalfile.filePath();
+    EXPECT_EQ(toPath(externalfile.fileName()), externalfile.filePath());
+
+    ForwardTranslator ft;
+    Workspace workspace = ft.translateModel(model);
+
+    std::vector<WorkspaceObject> objects = workspace.getObjectsByType(IddObjectType::Schedule_File);
+    ASSERT_EQ(1u, objects.size());
+
+    boost::optional<std::string> fileName = objects[0].getString(2);  // File Name
+    ASSERT_TRUE(fileName);
+    EXPECT_TRUE(toPath(fileName.get()).is_relative());  // rel path
+    EXPECT_EQ(externalfile.filePath(), fileName.get());
+  }
+}
diff --git a/src/model/ScheduleFile.cpp b/src/model/ScheduleFile.cpp
index 44cd842f0e5..5649fb17b29 100644
--- a/src/model/ScheduleFile.cpp
+++ b/src/model/ScheduleFile.cpp
@@ -233,6 +233,16 @@ namespace model {
       return csvFile;
     }
 
+    bool ScheduleFile_Impl::translateFileWithRelativePath() const {
+      boost::optional<std::string> value = getString(OS_Schedule_FileFields::TranslateFileWithRelativePath, true);
+      OS_ASSERT(value);
+      return openstudio::istringEqual(value.get(), "Yes");
+    }
+
+    bool ScheduleFile_Impl::isTranslateFileWithRelativePathDefaulted() const {
+      return isEmpty(OS_Schedule_FileFields::TranslateFileWithRelativePath);
+    }
+
     /* FIXME!
   openstudio::TimeSeries ScheduleFile_Impl::timeSeries(unsigned columnIndex) const
   {
@@ -376,6 +386,36 @@ namespace model {
     }*/
     }
 
+    bool ScheduleFile_Impl::setTranslateFileWithRelativePath(bool translateFileWithRelativePath) {
+      bool result = false;
+      if (translateFileWithRelativePath) {
+        result = setString(OS_Schedule_FileFields::TranslateFileWithRelativePath, "Yes");
+      } else {
+        result = setString(OS_Schedule_FileFields::TranslateFileWithRelativePath, "No");
+      }
+      OS_ASSERT(result);
+      return result;
+    }
+
+    void ScheduleFile_Impl::resetTranslateFileWithRelativePath() {
+      const bool result = setString(OS_Schedule_FileFields::TranslateFileWithRelativePath, "");
+      OS_ASSERT(result);
+    }
+
+    openstudio::path ScheduleFile_Impl::translatedFilePath() const {
+      if (translateFileWithRelativePath()) {
+        return toPath(externalFile().fileName());
+      }
+      openstudio::path filePath = externalFile().filePath();
+      if (!exists(filePath)) {
+        LOG(Warn, "Cannot find file \"" << filePath << "\"");
+      } else {
+        // make the path correct for this system
+        filePath = system_complete(filePath);
+      }
+      return filePath;
+    }
+
   }  // namespace detail
 
   ScheduleFile::ScheduleFile(const ExternalFile& externalfile, int column, int rowsToSkip)
@@ -477,6 +517,14 @@ namespace model {
     return getImpl<detail::ScheduleFile_Impl>()->csvFile();
   }
 
+  bool ScheduleFile::translateFileWithRelativePath() const {
+    return getImpl<detail::ScheduleFile_Impl>()->translateFileWithRelativePath();
+  }
+
+  bool ScheduleFile::isTranslateFileWithRelativePathDefaulted() const {
+    return getImpl<detail::ScheduleFile_Impl>()->isTranslateFileWithRelativePathDefaulted();
+  }
+
   /* FIXME!
 openstudio::TimeSeries ScheduleFile::timeSeries(unsigned columnIndex) const {
   return getImpl<detail::ScheduleFile_Impl>()->timeSeries(columnIndex);
@@ -551,6 +599,18 @@ unsigned ScheduleFile::addTimeSeries(const openstudio::TimeSeries& timeSeries) {
     getImpl<detail::ScheduleFile_Impl>()->resetAdjustScheduleforDaylightSavings();
   }
 
+  bool ScheduleFile::setTranslateFileWithRelativePath(bool translateFileWithRelativePath) {
+    return getImpl<detail::ScheduleFile_Impl>()->setTranslateFileWithRelativePath(translateFileWithRelativePath);
+  }
+
+  void ScheduleFile::resetTranslateFileWithRelativePath() {
+    getImpl<detail::ScheduleFile_Impl>()->resetTranslateFileWithRelativePath();
+  }
+
+  openstudio::path ScheduleFile::translatedFilePath() const {
+    return getImpl<detail::ScheduleFile_Impl>()->translatedFilePath();
+  }
+
   /// @cond
   ScheduleFile::ScheduleFile(std::shared_ptr<detail::ScheduleFile_Impl> impl) : ScheduleInterval(std::move(impl)) {}
   /// @endcond
diff --git a/src/model/ScheduleFile.hpp b/src/model/ScheduleFile.hpp
index 528611f1309..9d95b07a27e 100644
--- a/src/model/ScheduleFile.hpp
+++ b/src/model/ScheduleFile.hpp
@@ -86,6 +86,10 @@ namespace model {
 
     boost::optional<CSVFile> csvFile() const;
 
+    bool translateFileWithRelativePath() const;
+
+    bool isTranslateFileWithRelativePathDefaulted() const;
+
     //@}
     /** @name Setters */
     //@{
@@ -120,10 +124,17 @@ namespace model {
 
     /* FIXME! unsigned addTimeSeries(const openstudio::TimeSeries& timeSeries); */
 
+    bool setTranslateFileWithRelativePath(bool translateFileWithRelativePath);
+
+    void resetTranslateFileWithRelativePath();
+
     //@}
     /** @name Other */
     //@{
 
+    // Depending on the value of 'Translate File With Relative Path', returns an absolute or a relative path
+    openstudio::path translatedFilePath() const;
+
     //@}
    protected:
     /// @cond
diff --git a/src/model/ScheduleFile_Impl.hpp b/src/model/ScheduleFile_Impl.hpp
index 6a68b8287b0..5c125645bf0 100644
--- a/src/model/ScheduleFile_Impl.hpp
+++ b/src/model/ScheduleFile_Impl.hpp
@@ -88,6 +88,10 @@ namespace model {
 
       boost::optional<CSVFile> csvFile() const;
 
+      bool translateFileWithRelativePath() const;
+
+      bool isTranslateFileWithRelativePathDefaulted() const;
+
       //@}
       /** @name Setters */
       //@{
@@ -121,10 +125,16 @@ namespace model {
       // ensure that this object does not contain the date 2/29
       virtual void ensureNoLeapDays() override;
 
+      bool setTranslateFileWithRelativePath(bool translateFileWithRelativePath);
+
+      void resetTranslateFileWithRelativePath();
+
       //@}
       /** @name Other */
       //@{
 
+      openstudio::path translatedFilePath() const;
+
       //@}
      protected:
      private:


Test patch:
diff --git a/src/model/test/ScheduleInterval_GTest.cpp b/src/model/test/ScheduleInterval_GTest.cpp
index 9364696eadd..bdee273366e 100644
--- a/src/model/test/ScheduleInterval_GTest.cpp
+++ b/src/model/test/ScheduleInterval_GTest.cpp
@@ -320,6 +320,19 @@ TEST_F(ModelFixture, ScheduleFile) {
   schedule3.resetAdjustScheduleforDaylightSavings();
   EXPECT_TRUE(schedule3.isAdjustScheduleforDaylightSavingsDefaulted());
 
+  EXPECT_FALSE(schedule3.translateFileWithRelativePath());
+  EXPECT_TRUE(schedule3.isTranslateFileWithRelativePathDefaulted());
+  EXPECT_EQ(externalfile->filePath(), schedule3.translatedFilePath());
+
+  EXPECT_TRUE(schedule3.setTranslateFileWithRelativePath(true));
+  EXPECT_TRUE(schedule3.translateFileWithRelativePath());
+  EXPECT_FALSE(schedule3.isTranslateFileWithRelativePathDefaulted());
+  EXPECT_EQ(toPath(externalfile->fileName()), schedule3.translatedFilePath());
+
+  schedule3.resetTranslateFileWithRelativePath();
+  EXPECT_FALSE(schedule3.translateFileWithRelativePath());
+  EXPECT_TRUE(schedule3.isTranslateFileWithRelativePathDefaulted());
+
   // shouldn't create a new object
   boost::optional<ExternalFile> externalfile2 = ExternalFile::getExternalFile(model, openstudio::toString(p));
   ASSERT_TRUE(externalfile2);