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

5ZoneTDV example file fatal due to incorrect path to schedule file in Ep-Launch V9.6 #9164

Closed
3 tasks
Nigusse opened this issue Nov 2, 2021 · 9 comments · Fixed by #9321
Closed
3 tasks
Assignees

Comments

@Nigusse
Copy link
Contributor

Nigusse commented Nov 2, 2021

Issue overview

5ZoneTDV example file fatal out when run using EP-Launch Version 9.6 when trying to access Schedule:File object. EP-Launch is looking at incorrect path to the file. This file and others run fine in EnergyPlus V9.5. But it runs fine in V9.6 if you provide full path to the file in Schedule:File object.

Details

Some additional details for this issue (if relevant):

  • EnergyPlus V9.6 (EP-Launch)

Error Message:

Severe  ** Schedule:File="ELECTDVFROMCZ06COM", File Name: "..\datasets\TDV\TDV_2008_kBtu_CTZ06.csv" not found.
   **   ~~~   **   Paths searched:
   **   ~~~   **     Current Working Directory: "C:\Data\EPlusPrograms\EnergyPlusV9-6-0\ExampleFiles\datasets\TDV"
   **   ~~~   **     EnergyPlus Executable Directory: "C:\Data\EPlusPrograms\datasets\TDV"
...
Fatal  ** ProcessScheduleInput: Preceding Errors cause termination.

EP-Launch should be looking at C:\Data\EPlusPrograms\EnergyPlusV9-6-0\datasets\TDV\ instead of C:\Data\EPlusPrograms\EnergyPlusV9-6-0\ExampleFiles\datasets\TDV.

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Defect file added (list location of defect file here)
  • Ticket added to Pivotal for defect (development team task)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@mjwitte
Copy link
Contributor

mjwitte commented Nov 2, 2021

@Nigusse Nothing changed with EP-Launch. EnergyPlus does the searching for this. Something must have changed in the search path list in EnergyPlus between 9.5 and 9.6.

@mjwitte
Copy link
Contributor

mjwitte commented Feb 28, 2022

Tested with v9.5 on Windows10 with a bad file name to see what search paths were checked:

   ** Severe  ** Schedule:File="ELECTDVFROMCZ06COM", File Name: "..\datasets\TDV\TDV_XYZ2008_kBtu_CTZ06.csv" not found.
   **   ~~~   **   Paths searched:
   **   ~~~   **     Current Working Directory: "C:\EnergyPlus\EnergyPlusV9-5-0\ExampleFiles\datasets\TDV\"
   **   ~~~   **     EnergyPlus Executable Directory: "C:\EnergyPlus\datasets\TDV\"
   **   ~~~   **     "epin" Environment Variable: "C:\EnergyPlus\EnergyPlusV9-5-0\datasets\TDV\"

Now with v22.1-IOFreeze'ish

   ** Severe  ** Schedule:File="ELECTDVFROMCZ06COM", File Name: "..\datasets\TDV\TDV_2008_kBtu_CTZ06.csv" not found.
   **   ~~~   **   Paths searched:
   **   ~~~   **     Current Working Directory: "C:\EnergyPlus\EnergyPlusV22-1-0-IOFreeze\ExampleFiles\datasets\TDV"
   **   ~~~   **     EnergyPlus Executable Directory: "C:\datasets\TDV"

I see 2 differences here:

  1. The EnergyPlus Executable Directory path is wrong. The exe is in C:\EnergyPlus\EnergyPlusV22-1-0-IOFreeze, so, when combined with the file path of ..\datasets\TDV, the search path should be "C:\EnergyPlus\datasets\TDV", same as v9.5. (I realize that this is still not going to find the file but if someone is relying on putting things relative to the exe path, that appears to be broken, on Windows anyway.
  2. The "epin" Environment Variable path is no longer being searched.

@jcyuan2020
Copy link
Contributor

The problem might be here:

std::string ShadingSunlitFracFileName = Alphas(1);
std::string contextString = CurrentModuleObject + ", " + cAlphaFields(1) + ": ";
state.files.TempFullFilePath.filePath = CheckForActualFilePath(state, ShadingSunlitFracFileName, contextString);

The function CheckForActualFilePath() need to take std:filesystem::path type as the second parameter; however a std:string was supplied. The code lines might need to be changed to:

            // std::string ShadingSunlitFracFileName = Alphas(1);
            std::filesystem::path ShadingSunlitFracFileName = Alphas(1);

            std::string contextString = CurrentModuleObject + ", " + cAlphaFields(1) + ": ";
            state.files.TempFullFilePath.filePath = CheckForActualFilePath(state, ShadingSunlitFracFileName, contextString);

Another place that need to change is here:

state.files.TempFullFilePath.filePath = CheckForActualFilePath(state, Alphas(3), contextString);

This line should be changed to the following:

            std::filesystem::path alphas3_path = Alphas(3);
            // state.files.TempFullFilePath.filePath = CheckForActualFilePath(state, Alphas(3), contextString);
            state.files.TempFullFilePath.filePath = CheckForActualFilePath(state, alphas3_path, contextString);

There seems to be a difference about std::filesystem::path and std::string, according to the findings in #9313 which seems to have a similar problem.

There are about 5-6 calls for CheckForAcutalFilePath() in the entire code. All the others calls seem to correct--having the second parameter as std:filesystem::path type. The two occurrences above in the ScheduleManager.cc should be changed accordingly.

@jcyuan2020 jcyuan2020 self-assigned this Mar 3, 2022
@mbadams5
Copy link
Contributor

mbadams5 commented Mar 3, 2022

It should already implicitly construct a std::filesystem::path for the second parameter when given a std::string as an input parameter since std::filesystem::path accepts std::string as a constructor option (https://en.cppreference.com/w/cpp/filesystem/path/path). So I am doubtful that will solve this problem, but I could be wrong.

I think this is due to the original change to using std::filesystem and something being subtly wrong during the conversion.

@jmarrec
Copy link
Contributor

jmarrec commented Mar 3, 2022

@jcyuan2020 are you already actively looking into this? Otherwise I would take it, bundle it with #9313 (since I did the filesystem conversion, both are my fault...)

FYI, the relevant code block is around here:

std::vector<std::pair<fs::path, std::string>> pathsChecked;
const std::array<std::pair<fs::path, std::string>, 7> pathsToCheck = {
{{InputFilePath, "Current Working Directory"},
{state.dataStrGlobals->inputDirPath / InputFilePath, "IDF Directory"},
{state.dataStrGlobals->exeDirectoryPath / InputFilePath, "EnergyPlus Executable Directory"},
{state.dataSysVars->envinputpath1 / InputFilePath, "\"epin\" Environment Variable"},
{state.dataSysVars->envinputpath2 / InputFilePath, "\"input_path\" Environment Variable"},
{state.dataStrGlobals->CurrentWorkingFolder / InputFilePath, "INI File Directory"},
{state.dataStrGlobals->ProgramPath / InputFilePath, "\"program\", \"dir\" from INI File"}}};
std::size_t numPathsToNotTest = (state.dataSysVars->TestAllPaths) ? pathsToCheck.size() - 2 : pathsToCheck.size();
for (std::size_t i = 0; i < numPathsToNotTest; i++) {
if (FileSystem::fileExists(pathsToCheck[i].first)) {
foundFilePath = pathsToCheck[i].first;
print(state.files.audit, "{}={}\n", "found (" + pathsToCheck[i].second + ")", FileSystem::getAbsolutePath(foundFilePath).string());
return foundFilePath;
} else {
std::pair<fs::path, std::string> currentPath(FileSystem::getParentDirectoryPath(FileSystem::getAbsolutePath(pathsToCheck[i].first)),
pathsToCheck[i].second);
bool found = false;
for (auto path : pathsChecked) {
if (path.first == currentPath.first) {
found = true;
}
}
if (!found) {
pathsChecked.push_back(currentPath);
}
print(state.files.audit,
"{}={}\n",
"not found (" + pathsToCheck[i].second + ")\"",
FileSystem::getAbsolutePath(pathsToCheck[i].first).string());
}
}
// If we get here, we didn't find the file
ShowSevereError(state, contextString + "\"" + originalInputFilePath.string() + "\" not found.");
ShowContinueError(state, " Paths searched:");
for (auto path : pathsChecked) {
ShowContinueError(state, " " + path.second + ": \"" + path.first.string() + "\"");
}

It doesn't report a path as checked if that path doesn't exist.

I'm pretty sure this is an issue with a missing trailing directory separator...

@jcyuan2020 jcyuan2020 removed their assignment Mar 3, 2022
@jcyuan2020
Copy link
Contributor

@jmarrec I just removed myself from this one. You may have better idea to fix this one. Thanks!
@mbadams5 I did finished testing the changes I proposed and you are right. These are not the solutions to this issue.

@jmarrec jmarrec self-assigned this Mar 3, 2022
@jmarrec
Copy link
Contributor

jmarrec commented Mar 4, 2022

@jcyuan2020 Thanks.

Another potential issue with this... the folder datasets (in source) is installed as DataSets... Given than mac and linux are case sensitive file systems, not sure if that's a great idea.

@jmarrec
Copy link
Contributor

jmarrec commented Mar 4, 2022

yeah well, I can reproduce a similar error message on Ubuntu when installed, Ubuntu is case sensitive (mac isn't by default, including on my machine, so it works there).

$ EnergyPlus-22.1.0-b7de83fa2d-Linux-Ubuntu18.04-x86_64/energyplus -d out-5ZoneTDV/ -w EnergyPlus-22.1.0-b7de83fa2d-Linux-Ubuntu18.04-x86_64/WeatherData/USA_IL_Chicago-OHare.Intl.AP.725300_TMY3.epw EnergyPlus-22.1.0-b7de83fa2d-Linux-Ubuntu18.04-x86_64/ExampleFiles/5ZoneTDV.idf

EnergyPlus, Version 22.1.0-b7de83fa2d, YMD=2022.03.04 14:28
**FATAL:ProcessScheduleInput: Preceding Errors cause termination.
EnergyPlus Run Time=00hr 00min  0.82sec

If I replace in 5ZoneTDV.idf:

-    ..\datasets\TDV\TDV_2008_kBtu_CTZ06.csv,  !- File Name
+    ..\DataSets\TDV\TDV_2008_kBtu_CTZ06.csv,  !- File Name

then it works.

@jmarrec
Copy link
Contributor

jmarrec commented Mar 8, 2022

I finally found the needle.

epin is passed as a a path to the IDF file minus its extension. eg : C:\EnergyPlus-V22-1-0\ExampleFiles\5ZoneTDV, so need to take the parent directory path like it was done before

get_environment_variable(cInputPath1, state.dataSysVars->envinputpath1);
if (!state.dataSysVars->envinputpath1.empty()) {
pos = index(state.dataSysVars->envinputpath1, pathChar, true); // look backwards for pathChar
if (pos != std::string::npos) state.dataSysVars->envinputpath1.erase(pos + 1);
}
.

I missed it when I did the conversion to filesystem...

get_environment_variable(cInputPath1, tmp);
state.dataSysVars->envinputpath1 = fs::path(tmp);

As a result, since EP-Launch creates an extra Temp subfolder to run it C:\EnergyPlus-V22-1-0\ExampleFiles\EPTEMP-00000001, and the epin wasn't set correctly, it couldn't find the ..\datasets\TDV\TDV_2008_kBtu_CTZ06.csv file.

mjwitte added a commit that referenced this issue Mar 21, 2022
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 a pull request may close this issue.

5 participants