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

Fix file not found output message #8324

Merged
merged 22 commits into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c4f656d
Create a vector to track locations tested for a file while searching …
ajscimone Sep 16, 2020
80e4d8f
Merge branch 'develop' into fix-file-not-found-output-message
ajscimone Sep 16, 2020
2ab0ac8
Merge branch 'develop' into fix-file-not-found-output-message
ajscimone Sep 29, 2020
22126fc
Refactor the CheckForActualFileName function to loop through an array…
ajscimone Sep 30, 2020
578d433
Add a unit test for the CheckForActualFileName function which catches…
ajscimone Sep 30, 2020
8f6e5ea
Merge branch 'develop' into fix-file-not-found-output-message
ajscimone Sep 30, 2020
8a3db61
Add a flag to file checking function to output error messages if the …
ajscimone Sep 30, 2020
75cd34d
Fix output error where printing of file searching errors was always p…
ajscimone Sep 30, 2020
e91271b
Fix variable access issues in check for actual file.
ajscimone Sep 30, 2020
2f839eb
Refactor CheckForActualFileName, removing the parameter for passing a…
ajscimone Sep 30, 2020
31a59c4
Clean up the other uses of CheckForActualFileName and add context str…
ajscimone Sep 30, 2020
631eb86
Remove a few unneeded comments from the code base.
ajscimone Sep 30, 2020
11911ee
Clean up some issues with the file not found unit test, making it wor…
ajscimone Sep 30, 2020
62d43df
Merge branch 'develop' into fix-file-not-found-output-message
ajscimone Sep 30, 2020
f04d3a3
Add the variable being searched through to the output string.
ajscimone Oct 1, 2020
7d82651
Reformat the output messages to the .err file for file searching file…
ajscimone Oct 1, 2020
b0ed48c
Fix the expected error in the file not found unit test.
ajscimone Oct 1, 2020
296a9b9
Merge develop.
ajscimone Oct 14, 2020
5438bb1
Merge develop.
nealkruis Nov 9, 2020
4a93d7e
Remove unused environment variable.
nealkruis Nov 11, 2020
f1bcbbd
Convert to std::array.
nealkruis Nov 11, 2020
fbdcded
Fix error message line break.
nealkruis Nov 11, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/EnergyPlus/CurveManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1750,9 +1750,13 @@ namespace CurveManager {
std::size_t rowNum = indVarInstance.at("external_file_starting_row_number").get<std::size_t>() - 1;

if (!state.dataCurveManager->btwxtManager.tableFiles.count(filePath)) {
state.dataCurveManager->btwxtManager.tableFiles.emplace(filePath, TableFile{state, filePath});
TableFile tableFile;
ErrorsFound |= tableFile.load(state, filePath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking the initialization into a separate bool load() function makes sense.

state.dataCurveManager->btwxtManager.tableFiles.emplace(filePath, tableFile);
}

if (ErrorsFound) continue; // Unable to load file so continue on to see if there are other errors before fataling

axis = state.dataCurveManager->btwxtManager.tableFiles[filePath].getArray(state, {colNum, rowNum});

// remove NANs
Expand Down Expand Up @@ -1940,10 +1944,15 @@ namespace CurveManager {
std::size_t colNum = fields.at("external_file_column_number").get<std::size_t>() - 1;
std::size_t rowNum = fields.at("external_file_starting_row_number").get<std::size_t>() - 1;


if (!state.dataCurveManager->btwxtManager.tableFiles.count(filePath)) {
state.dataCurveManager->btwxtManager.tableFiles.emplace(filePath, TableFile{state, filePath});
TableFile tableFile;
ErrorsFound |= tableFile.load(state, filePath);
state.dataCurveManager->btwxtManager.tableFiles.emplace(filePath, tableFile);
}

if (ErrorsFound) continue; // Unable to load file so continue on to see if there are other errors before fataling

lookupValues = state.dataCurveManager->btwxtManager.tableFiles[filePath].getArray(state, {colNum, rowNum});

// remove NANs
Expand Down Expand Up @@ -2040,19 +2049,15 @@ namespace CurveManager {
tableFiles.clear();
}

TableFile::TableFile(EnergyPlusData &state, std::string path)
{
load(state, path);
}

void TableFile::load(EnergyPlusData &state, std::string path)
bool TableFile::load(EnergyPlusData &state, std::string path)
{
filePath = path;
bool fileFound;
std::string fullPath;
DataSystemVariables::CheckForActualFileName(state, path, fileFound, fullPath);
if (!fileFound) {
ShowFatalError(state, "File \"" + filePath + "\" : File not found.");
std::string contextString = "CurveManager::TableFile::load: ";
DataSystemVariables::CheckForActualFileName(state, path, fileFound, fullPath, contextString);
if(!fileFound){
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So true means error. OK. A little confusing maybe but not a big deal. If other commits are needed here for anything, I'd suggest considering a clearer convention like bool loadSuccess() where true is clearly success. Anyway, not a big deal if it's working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was kind of thinking of it as an error code return where 0/false means success.

}
std::ifstream file(fullPath);
std::string line("");
Expand Down Expand Up @@ -2086,6 +2091,7 @@ namespace CurveManager {
++colNum;
}
}
return false;
}

std::vector<double>& TableFile::getArray(EnergyPlusData &state, std::pair<std::size_t, std::size_t> colAndRow) {
Expand Down
2 changes: 1 addition & 1 deletion src/EnergyPlus/CurveManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ namespace CurveManager {
std::string filePath;
std::vector<std::vector<std::string>> contents;
std::map<std::pair<std::size_t, std::size_t>, std::vector<double>> arrays;
void load(EnergyPlusData &state, std::string path);
bool load(EnergyPlusData &state, std::string path);
std::vector<double>& getArray(EnergyPlusData &state, std::pair<std::size_t, std::size_t> colAndRow);

private:
Expand Down
129 changes: 44 additions & 85 deletions src/EnergyPlus/DataSystemVariables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.

//C++ Headers
#include <utility>

// ObjexxFCL Headers
#include <ObjexxFCL/environment.hh>
#include <ObjexxFCL/string.functions.hh>
Expand Down Expand Up @@ -200,7 +203,8 @@ namespace DataSystemVariables {
void CheckForActualFileName(EnergyPlusData &state,
std::string const &originalInputFileName, // name as input for object
bool &FileFound, // Set to true if file found and is in CheckedFileName
std::string &CheckedFileName // Blank if not found.
std::string &CheckedFileName, // Blank if not found.
const std::string contextString //
)
{

Expand All @@ -215,26 +219,11 @@ namespace DataSystemVariables {
// be accurate. This searches a few folders (CurrentWorkingFolder, Program folder) to see
// if the file can be found. (It may have been input with full path so that is checked first.)

// METHODOLOGY EMPLOYED:
// na

// REFERENCES:
// na

// USE STATEMENTS:
// na

// Locals
// SUBROUTINE ARGUMENT DEFINITIONS:

// SUBROUTINE PARAMETER DEFINITIONS:

// INTERFACE BLOCK SPECIFICATIONS:
// na

// DERIVED TYPE DEFINITIONS:
// na

// SUBROUTINE LOCAL VARIABLE DECLARATIONS:
std::string InputFileName; // save for changing out path characters
std::string::size_type pos;
Expand All @@ -251,81 +240,51 @@ namespace DataSystemVariables {
firstTime = false;
}


FileFound = false;
CheckedFileName.clear();
InputFileName = originalInputFileName;
makeNativePath(InputFileName);

if (FileSystem::fileExists(InputFileName)) {
FileFound = true;
CheckedFileName = InputFileName;
print(state.files.audit, "{}={}\n", "found (user input)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (user input)", getAbsolutePath(InputFileName));
}

// Look relative to input file path
if (FileSystem::fileExists(DataStringGlobals::inputDirPathName + InputFileName)) {
FileFound = true;
CheckedFileName = DataStringGlobals::inputDirPathName + InputFileName;
print(state.files.audit, "{}={}\n", "found (input file)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (input file)", getAbsolutePath(DataStringGlobals::inputDirPathName + InputFileName));
}

// Look relative to input path
if (FileSystem::fileExists(envinputpath1 + InputFileName)) {
FileFound = true;
CheckedFileName = envinputpath1 + InputFileName;
print(state.files.audit, "{}={}\n", "found (epin)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (epin)", getAbsolutePath(envinputpath1 + InputFileName));
}

// Look relative to input path
if (FileSystem::fileExists(envinputpath2 + InputFileName)) {
FileFound = true;
CheckedFileName = envinputpath2 + InputFileName;
print(state.files.audit, "{}={}\n", "found (input_path)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (input_path)", getAbsolutePath(envinputpath2 + InputFileName));
}

// Look relative to program path
if (FileSystem::fileExists(envprogrampath + InputFileName)) {
FileFound = true;
CheckedFileName = envprogrampath + InputFileName;
print(state.files.audit, "{}={}\n", "found (program_path)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (program_path)", getAbsolutePath(envprogrampath + InputFileName));
}

if (!TestAllPaths) return;

// Look relative to current working folder
if (FileSystem::fileExists(CurrentWorkingFolder + InputFileName)) {
FileFound = true;
CheckedFileName = CurrentWorkingFolder + InputFileName;
print(state.files.audit, "{}={}\n", "found (CWF)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (CWF)", getAbsolutePath(CurrentWorkingFolder + InputFileName));
std::vector<std::pair<std::string, std::string>> pathsChecked;

const std::vector<std::pair<std::string, std::string>> pathsToCheck = {
{InputFileName, "Current Working Directory"},
{DataStringGlobals::inputDirPathName + InputFileName, "IDF Directory"},
{DataStringGlobals::exeDirectory + InputFileName, "EnergyPlus Executable Directory"},
{envinputpath1 + InputFileName, "\"epin\" Environment Variable"},
{envinputpath2 + InputFileName, "\"input_path\" Environment Variable"},
{envprogrampath + InputFileName, "\"program_path\" Environment Variable"},
{CurrentWorkingFolder + InputFileName, "INI File Directory"},
{ProgramPath + InputFileName, "\"program\", \"dir\" from INI File"}
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a small list, so it's not a big deal, but an array would've been acceptable here since I think this vector is staying fixed length.


std::size_t numPathsToNotTest = (TestAllPaths) ? pathsToCheck.size()-2 : pathsToCheck.size();

for(std::size_t i = 0; i < numPathsToNotTest; i++) {
if (FileSystem::fileExists(pathsToCheck[i].first)) {
FileFound = true;
CheckedFileName = pathsToCheck[i].first;
print(state.files.audit, "{}={}\n", "found (" + pathsToCheck[i].second +")", getAbsolutePath(CheckedFileName));
return;
} else {
std::pair <std::string,std::string> currentPath(getParentDirectoryPath(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 +")\"", getAbsolutePath(pathsToCheck[i].first));
}
}

// Look relative to program path
if (FileSystem::fileExists(ProgramPath + InputFileName)) {
FileFound = true;
CheckedFileName = ProgramPath + InputFileName;
print(state.files.audit, "{}={}\n", "found (program path - ini)", getAbsolutePath(CheckedFileName));
return;
} else {
print(state.files.audit, "{}={}\n", "not found (program path - ini)", getAbsolutePath(ProgramPath + InputFileName));
if (!FileFound) {
ShowSevereError(state, contextString+ "\"" + originalInputFileName + "\" not found. Paths searched:");
for(auto path: pathsChecked){
ShowContinueError(state, " " + path.second +": \"" + path.first +"\"");
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/EnergyPlus/DataSystemVariables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

// EnergyPlus Headers
#include <EnergyPlus/EnergyPlus.hh>
#include <EnergyPlus/IOFiles.hh>

namespace EnergyPlus {

Expand Down Expand Up @@ -171,7 +172,8 @@ namespace DataSystemVariables {
void CheckForActualFileName(EnergyPlusData &state,
std::string const &originalInputFileName, // name as input for object
bool &FileFound, // Set to true if file found and is in CheckedFileName
std::string &CheckedFileName // Blank if not found.
std::string &foundFileName, // Blank if not found.
const std::string contextString = std::string()
);

// Needed for unit tests, should not be normally called.
Expand Down
8 changes: 5 additions & 3 deletions src/EnergyPlus/ExternalInterface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,11 @@ namespace ExternalInterface {
cNumericFieldNames);
// Get the FMU name
FMU(Loop).Name = cAlphaArgs(1);
CheckForActualFileName(state, cAlphaArgs(1), fileExist, tempFullFileName);

std::string contextString = cCurrentModuleObject + ", " + cAlphaFieldNames(1) + ": ";

CheckForActualFileName(state, cAlphaArgs(1), fileExist, tempFullFileName, contextString);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, just needing to add a new context string.


if (fileExist) {
pos = index(FMU(Loop).Name, pathChar, true); // look backwards
if (pos != std::string::npos) {
Expand All @@ -1165,8 +1169,6 @@ namespace ExternalInterface {
}
fullFileName(Loop) = tempFullFileName;
} else {
ShowSevereError(state, "ExternalInterface/InitExternalInterfaceFMUImport:");
ShowContinueError(state, "file not located = \"" + cAlphaArgs(1) + "\".");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then no longer needing to report the error details.

ErrorsFound = true;
}
// Get fmu time out
Expand Down
8 changes: 3 additions & 5 deletions src/EnergyPlus/HeatBalanceManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6294,14 +6294,12 @@ namespace HeatBalanceManager {
ConstructionFound = false;
// ErrorsFound = .FALSE.
EOFonFile = false;
std::string contextString = "HeatBalanceManager::SearchWindow5DataFile: ";

CheckForActualFileName(state, DesiredFileName, exists, state.files.TempFullFileName.fileName, contextString);

CheckForActualFileName(state, DesiredFileName, exists, state.files.TempFullFileName.fileName);
// INQUIRE(FILE=TRIM(DesiredFileName), EXIST=exists)
if (!exists) {
ShowSevereError(state, "HeatBalanceManager: SearchWindow5DataFile: Could not locate Window5 Data File, expecting it as file name=" +
DesiredFileName);
ShowContinueError(state, "Certain run environments require a full path to be included with the file name in the input field.");
ShowContinueError(state, "Try again with putting full path and file name in the field.");
ShowFatalError(state, "Program terminates due to these conditions.");
}

Expand Down
18 changes: 7 additions & 11 deletions src/EnergyPlus/ScheduleManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -533,12 +533,11 @@ namespace ScheduleManager {
inputProcessor->getObjectItem(
state, CurrentModuleObject, 1, Alphas, NumAlphas, Numbers, NumNumbers, Status, lNumericBlanks, lAlphaBlanks, cAlphaFields, cNumericFields);
std::string ShadingSunlitFracFileName = Alphas(1);
CheckForActualFileName(state, ShadingSunlitFracFileName, FileExists, state.files.TempFullFileName.fileName);

std::string contextString = CurrentModuleObject + ", " + cAlphaFields(1) + ": ";
CheckForActualFileName(state, ShadingSunlitFracFileName, FileExists, state.files.TempFullFileName.fileName, contextString);

if (!FileExists) {
ShowSevereError(state, RoutineName + ":\"" + ShadingSunlitFracFileName +
"\" not found when External Shading Calculation Method = ImportedShading.");
ShowContinueError(state, "Certain run environments require a full path to be included with the file name in the input field.");
ShowContinueError(state, "Try again with putting full path and file name in the field.");
ShowFatalError(state, "Program terminates due to previous condition.");
}

Expand Down Expand Up @@ -1758,16 +1757,13 @@ namespace ScheduleManager {
// ENDDO
// ENDIF

CheckForActualFileName(state, Alphas(3), FileExists, state.files.TempFullFileName.fileName);
std::string contextString = CurrentModuleObject + "=\"" + Alphas(1) + "\", " + cAlphaFields(3) + ": ";

CheckForActualFileName(state, Alphas(3), FileExists, state.files.TempFullFileName.fileName, contextString);

// INQUIRE(file=Alphas(3),EXIST=FileExists)
// Setup file reading parameters
if (!FileExists) {
DisplayString("Missing " + Alphas(3));
ShowSevereError(state, RoutineName + CurrentModuleObject + "=\"" + Alphas(1) + "\", " + cAlphaFields(3) + "=\"" + Alphas(3) +
"\" not found.");
ShowContinueError(state, "Certain run environments require a full path to be included with the file name in the input field.");
ShowContinueError(state, "Try again with putting full path and file name in the field.");
ErrorsFound = true;
} else {
auto SchdFile = state.files.TempFullFileName.try_open();
Expand Down
1 change: 1 addition & 0 deletions tst/EnergyPlus/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ set( test_src
Datasets.unit.cc
DataSizing.unit.cc
DataSurfaces.unit.cc
DataSystemVariables.unit.cc
DataZoneEquipment.unit.cc
DaylightingManager.unit.cc
DElightManager.unit.cc
Expand Down
Loading