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

Performance refactor for Schedule:File #8795

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c672e80
Move Schedule:Shading code to Schedule:File:Shading
jmythms May 25, 2021
7c3eada
Initial commit
jmythms May 25, 2021
f7345b3
clang format
jmythms May 25, 2021
3b0ad93
removed unused variables
jmythms May 25, 2021
e8759fb
rearrange code and better comments
jmythms May 26, 2021
572729f
Fixed rowCountFld position
jmythms May 26, 2021
c3d9613
Merge branch 'develop' into schedManGoZoom
jmythms May 26, 2021
ddad276
Clang format + remove temporary vars for debug
jmythms May 26, 2021
f68ec73
moved File:Shading + Overloads for ProcessNum + Added checks
jmythms May 29, 2021
d81d886
fixed unit tests
jmythms May 31, 2021
9ed3f00
clang format
jmythms May 31, 2021
88cc194
fix numerrors
jmythms Jun 1, 2021
43a26f3
moved stripped back
jmythms Jun 1, 2021
69518f2
Merge branch 'develop' into schedManGoZoom
jmythms Jun 1, 2021
70f3713
temp - added performance metric
jmythms Jun 1, 2021
ca7692e
added stress test files
jmythms Jun 1, 2021
0654156
moved timer
jmythms Jun 2, 2021
f06a6fc
removed time mesaurements
jmythms Jun 2, 2021
c1dc928
removed temp test files
jmythms Jun 2, 2021
5a4a046
Fixed many to one map problem + changed string_view to string
jmythms Jun 8, 2021
5a60d62
cleaned up comments + clang format
jmythms Jun 8, 2021
5a8a285
Merge branch 'develop' into schedManGoZoom
jmythms Jun 8, 2021
bd12208
fix unused var warning
jmythms Jun 22, 2021
f760a95
revert shading data.csv
jmythms Jun 22, 2021
c2c88f8
merge develop + conflict resolution
jmythms Jun 22, 2021
cc095be
conflict resolution
jmythms Jun 22, 2021
b0acb76
removed ProcessNumber overload
jmythms Jun 22, 2021
6d21e92
removed string_view implementation in string.functions.hh
jmythms Jun 22, 2021
5e6ef79
removed all string_view changes
jmythms Jun 22, 2021
fe51879
revert SolarShadingTest.csv
jmythms Jun 22, 2021
5e8ed20
Merge branch 'develop' into schedManGoZoom
jmythms Jun 23, 2021
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
719 changes: 391 additions & 328 deletions src/EnergyPlus/ScheduleManager.cc

Large diffs are not rendered by default.

113 changes: 113 additions & 0 deletions src/EnergyPlus/ScheduleManager.hh
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@
#ifndef ScheduleManager_hh_INCLUDED
#define ScheduleManager_hh_INCLUDED

// C++ Headers
#include <set>

// ObjexxFCL Headers
#include <ObjexxFCL/Array1D.hh>
#include <ObjexxFCL/Array1S.hh>
Expand Down Expand Up @@ -328,6 +331,109 @@ namespace ScheduleManager {

int GetNumberOfSchedules(EnergyPlusData &state);

struct CSVRow
{
std::string_view operator[](std::size_t index) const;
std::size_t rowEnd();
std::string delimiter{'*'}; // eg. comma in CSV
void readNextRow(std::istream &str);

private:
std::string m_line;
std::vector<int> m_data;
};

std::istream &operator>>(std::istream &str, CSVRow &data);

void PreProcessIDF(EnergyPlus::EnergyPlusData &state, int &SchNum, int NumCommaFileSchedules);

struct schedInputIdfPreprocessObject
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Objects of this struct will hold the IDF file info about a particular Schedule:File that is read by the getObjectItem function. Implementing it like this hoping to bring a more object-oriented approach to this module.

{
std::string name; // <- can inherit from parent sched class
std::string fileName; // <- can inherit from parent sched class
int ScheduleTypePtr{-1};
int columnOfInterest{-1};
int numHourlyValues{-1};
int MinutesPerItem{-1};
int rowsToSkip{-1};
bool FileIntervalInterpolated{false};
std::basic_string<char> columnSeperator{"*"};
int columnarDataIndex{-1}; // TODO : Figure out what to do here (size_t to int conversion)
int SchNum{-1};

schedInputIdfPreprocessObject(std::string name, // <- can inherit from parent sched class
std::string fileName, // <- can inherit from parent sched class
int ScheduleTypePtr = -1,
int columnOfInterest = -1,
int numHourlyValues = -1,
int MinutesPerItem = -1,
int rowsToSkip = -1,
bool FileIntervalInterpolated = false,
std::basic_string<char> columnSeperator = "*",
int SchNum = -1)
{

this->name = name;
this->fileName = fileName;
this->ScheduleTypePtr = ScheduleTypePtr;
this->columnOfInterest = columnOfInterest - 1; // - 1 to get index to match with c++ containers (start at 0)
this->numHourlyValues = numHourlyValues;
this->MinutesPerItem = MinutesPerItem;
this->rowsToSkip = rowsToSkip;
this->FileIntervalInterpolated = FileIntervalInterpolated;
this->columnSeperator = columnSeperator;
this->SchNum = SchNum;
}
void setColumnarDataIndex(int incomingData)
{
this->columnarDataIndex = incomingData;
}
};

void PopulateSetOfFilenames(const std::vector<schedInputIdfPreprocessObject> &allIdfSchedData, std::set<std::string> &setOfFilenames);

struct PreProcessedColumn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Objects of this struct will have a vector (vals) that holds the actual schedule values from the CSV file.

{
std::string name;
int columnOfInterest = -1;
int numHourlyValues = -1;
int MinutesPerItem = -1;
int rowsToSkip = -1;
bool FileIntervalInterpolated = false;
int ScheduleTypePtr = -1;
int SchNum = -1;
int rowCnt = 0;
int numerrors = 0;
std::string fileName;
std::string delimiter;
std::vector<Real64> vals; // rows to skip + number of hours*items/hour

PreProcessedColumn(std::string name, // <- can inherit from parent sched class
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We implement constructors here, so emplace_back can be called, which is better than push_back because creating a copy is avoided.

int columnOfInterest,
int numHourlyValues,
int MinutesPerItem,
int rowsToSkip,
bool FileIntervalInterpolated,
std::string fileName,
std::string delimiter,
int ScheduleTypePtr,
int SchNum,
std::vector<Real64> vals)
{
this->name = name; // <- can inherit from parent sched class
this->columnOfInterest = columnOfInterest;
this->numHourlyValues = numHourlyValues;
this->MinutesPerItem = MinutesPerItem;
this->rowsToSkip = rowsToSkip;
this->FileIntervalInterpolated = FileIntervalInterpolated;
this->fileName = fileName;
this->delimiter = delimiter;
this->ScheduleTypePtr = ScheduleTypePtr;
this->SchNum = SchNum;
this->vals = vals;
}
};

} // namespace ScheduleManager

struct ScheduleManagerData : BaseGlobalStruct
Expand Down Expand Up @@ -355,6 +461,10 @@ struct ScheduleManagerData : BaseGlobalStruct
Array1D<ScheduleManager::WeekScheduleData> WeekSchedule; // Week Schedule Storage
Array1D<ScheduleManager::ScheduleData> Schedule; // Schedule Storage

// Schedule:File variables
std::vector<ScheduleManager::schedInputIdfPreprocessObject> allIdfSchedData;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A vector of schedInputIdfPreprocessObject type. We will add the IDF data about each Schedule:File object to this vector as schedInputIdfPreprocessObject elements.

std::vector<ScheduleManager::PreProcessedColumn> columnarData;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A vector of PreProcessedColumn objects. This will hold the schedule data that will be later inserted into the schedule data structure.


void clear_state() override
{
CheckScheduleValueMinMaxRunOnceOnly = true;
Expand All @@ -376,6 +486,9 @@ struct ScheduleManagerData : BaseGlobalStruct
DaySchedule.clear(); // Day Schedule Storage
WeekSchedule.clear(); // Week Schedule Storage
Schedule.clear(); // Schedule Storage

allIdfSchedData.clear();
columnarData.clear();
}
};

Expand Down
23 changes: 23 additions & 0 deletions src/EnergyPlus/StringUtilities.hh
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ inline std::stringstream stringReader(std::string str)
return result;
}

inline std::stringstream stringReader(std::string_view str)
{
std::stringstream ss;
ss << str;
ss.imbue(std::locale("C")); // the forced locale is to avoid issues with parsing floating point numbers that use non-decimal formats
return ss;
}

template <typename Param> bool readListItem(std::istream &stream, Param &&param)
{
if (stream.good()) {
Expand All @@ -80,6 +88,21 @@ template <typename Param> bool readItem(std::string input, Param &&param)
{
auto stream = stringReader(std::move(input));
stream >> param;

return !stream.fail() && stream.eof();
}

template <typename Param> bool readItem(std::string_view input, Param &&param)
{
std::string temp_string = static_cast<std::string>(input);
// make FORTRAN floating point number (containing 'd' or 'D')
// standardized by replacing 'd' or 'D' with 'e'
std::replace_if(
std::begin(temp_string), std::end(temp_string), [](const char c) { return c == 'D' || c == 'd'; }, 'e');
auto stream = stringReader(temp_string);

stream >> param;

return !stream.fail() && stream.eof();
}

Expand Down
58 changes: 57 additions & 1 deletion src/EnergyPlus/UtilityRoutines.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ namespace UtilityRoutines {
// List directed Fortran input/output.

// SUBROUTINE PARAMETER DEFINITIONS:
static std::string const ValidNumerics("0123456789.+-EeDd");
static constexpr std::string_view ValidNumerics("0123456789.+-EeDd");

Real64 rProcessNumber = 0.0;
// Make sure the string has all what we think numerics should have
Expand Down Expand Up @@ -151,6 +151,62 @@ namespace UtilityRoutines {
return rProcessNumber;
}

Real64 ProcessNumber(std::basic_string_view<char> const &String_view, bool &ErrorFlag)
{

// FUNCTION INFORMATION:
// AUTHOR Linda K. Lawrie
// DATE WRITTEN September 1997
// MODIFIED na
// RE-ENGINEERED na

// PURPOSE OF THIS FUNCTION:
// This function processes a string that should be numeric and
// returns the real value of the string.

// METHODOLOGY EMPLOYED:
// FUNCTION ProcessNumber translates the argument (a string)
// into a real number. The string should consist of all
// numeric characters (except a decimal point). Numerics
// with exponentiation (i.e. 1.2345E+03) are allowed but if
// it is not a valid number an error message along with the
// string causing the error is printed out and 0.0 is returned
// as the value.

// REFERENCES:
// List directed Fortran input/output.

// SUBROUTINE PARAMETER DEFINITIONS:
static constexpr std::string_view ValidNumerics("0123456789.+-EeDd");

Real64 rProcessNumber = 0.0;
// Make sure the string has all what we think numerics should have
std::string_view PString(stripped(String_view));
std::string::size_type const StringLen(PString.length());
ErrorFlag = false;
if (StringLen == 0) return rProcessNumber;
bool parseFailed = false;
if (PString.find_first_not_of(ValidNumerics) == std::string::npos) {

// std::replace_if - > moved to readItem
// is this not working because string_view is acting like a const variable? // Do we need to convert/cast it here? -> move this to the
// 'deepest' function so that the copying can be minimized

// then parse as a normal floating point value
parseFailed = !readItem(PString, rProcessNumber);
ErrorFlag = false;
} else {
rProcessNumber = 0.0;
ErrorFlag = true;
}
if (parseFailed) {
rProcessNumber = 0.0;
ErrorFlag = true;
}

return rProcessNumber;
}

int FindItemInList(std::string const &String, Array1_string const &ListOfItems, int const NumItems)
{

Expand Down
1 change: 1 addition & 0 deletions src/EnergyPlus/UtilityRoutines.hh
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ namespace UtilityRoutines {
};

Real64 ProcessNumber(std::string const &String, bool &ErrorFlag);
Real64 ProcessNumber(std::basic_string_view<char> const &String_view, bool &ErrorFlag);

int FindItemInList(std::string const &String, Array1_string const &ListOfItems, int NumItems);

Expand Down
16 changes: 16 additions & 0 deletions third_party/ObjexxFCL/src/ObjexxFCL/string.functions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,22 @@ stripped( std::string const & s )
}
}

std::string_view
stripped( std::string_view const & s )
{
if ( s.empty() ) {
return s;
} else {
std::string::size_type const ib( s.find_first_not_of( ' ' ) );
std::string::size_type const ie( s.find_last_not_of( ' ' ) );
if ( ( ib == std::string::npos ) || ( ie == std::string::npos ) ) { // All of string is ' '
return std::string(); // Return empty string
} else {
return s.substr( ib, ie - ib + 1 );
}
}
}

// Space Stripped from a string's Left Tail Copy of a string
std::string
lstripped( std::string const & s )
Expand Down
3 changes: 3 additions & 0 deletions third_party/ObjexxFCL/src/ObjexxFCL/string.functions.hh
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,9 @@ rstripped( std::string const & s, std::string const & chars );
std::string
stripped( std::string const & s );

std::string_view
stripped( std::string_view const & s );

// Space Stripped from a string's Left Tail Copy of a string
std::string
lstripped( std::string const & s );
Expand Down
19 changes: 16 additions & 3 deletions tst/EnergyPlus/unit/StringUtilities.unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,29 @@ TEST_F(EnergyPlusFixture, readItem)
int i = 0;

// should read just 12
EXPECT_TRUE(EnergyPlus::readItem("12", i));
EXPECT_TRUE(EnergyPlus::readItem(static_cast<std::string>("12"), i));
EXPECT_EQ(i, 12);

// should fail if unable to process entire input string
EXPECT_FALSE(EnergyPlus::readItem("1234fgq", i));
EXPECT_FALSE(EnergyPlus::readItem(static_cast<std::string>("1234fgq"), i));
// value is in an unknown state if read failed
// EXPECT_EQ(i, 12);

// should read nothing
EXPECT_FALSE(EnergyPlus::readItem("abc123", i));
EXPECT_FALSE(EnergyPlus::readItem(static_cast<std::string>("abc123"), i));
EXPECT_EQ(i, 0);

// should read just 12
EXPECT_TRUE(EnergyPlus::readItem(static_cast<std::string_view>("12"), i));
EXPECT_EQ(i, 12);

// should fail if unable to process entire input string
EXPECT_FALSE(EnergyPlus::readItem(static_cast<std::string_view>("1234fgq"), i));
// value is in an unknown state if read failed
// EXPECT_EQ(i, 12);

// should read nothing
EXPECT_FALSE(EnergyPlus::readItem(static_cast<std::string_view>("abc123"), i));
EXPECT_EQ(i, 0);
}

Expand Down