Performance refactor for Schedule:File

[jmythms]

Pull request overview

This pull request refactors the Schedule:File module in ScheduleManager.cc mainly focusing on improving run times.

The current Schedule:File module mainly has three steps:

1. Get input from IDF.
2. Parse CSV according to input from IDF from the last step.
3. Save parsed information to schedule data structure.

Out of these three, parsing the CSV is the most expensive step, because it involves reading information from potentially very big external files. This step also has the potential for generating maximum performance gain with a refactor, mainly because of problems with:

1. Repeating the file open-close operations for each Schedule:File object in the IDF
2. Repeating the unnecessary delimiter checks each time the schedule is run

We address both of these problems in one step by reading in information from the CSV file in one step. This is explained below:

Methodology

Current Method as in Develop:

It parses all Schedule:File in the following way:

1. For each Schedule:File Object, it
2. Opens file
3. Parses each line, scans the line looking for the specific column
4. Once it finds the correct column, it converts the token in that cell to a Real64 value and saves it to an array
5. Repeat till end of file
6. Close file
7. Repeat for next Schedule:File object in IDF

Fig 1.

This is how it looks in the above example:

0. Open file
1. Skip header row
2. Read the second line, process 1, skip 2,3,4,5,
3. Read the third line, process 6, skip 7,8,9,10
4. Repeat till end of file
5. Close file
6. Open the file again.
7. Skip header row
8. Read the second line, skip 1, process 2, skip 3,4,5
9. Read the third line, skip 6, process 7, skip 8,9,10
10. Repeat till end of file
11. Close file...

Repeat till all Schedule:File objects are processed

Proposed Method:

1. Scan IDF for all files associated with Schedule:File
2. For each file:
3. Find the schedule:File objects associated with it.
4. Open file
5. Read the row, process all tokens of interest in that specific row in the file to Real64.
6. Repeat for every row
7. Go till the end of the file
8. Repeat for every file

This is how it looks for the example:
0. Open file

1. Skip header row
2. Read first line, process 1,2,3,4,5
3. Read second line, process 6,7,8,9,10
4. Repeat for every row
5. Close file
6. Repeat for any other file

Advantages of this method:

1.For IDFs that have a large number of Schedule:File objects, decrease file open and file close operations significantly. These are very expensive IO operations.
2. Decrease the number of rows read, delimiter checks from N*(N+1)/2 to N. Where N is the number of columns in a file.

std::string_view

I tried to use std::string_view for the CSV parse part, since that will be the operation that is repeated most and it would be great to cut down the dynamic allocations made by string arrays. But I had to cast it to a string at some point, since replace_if needs a not-read-only object to replace things.

Performance Improvements

Percent SpeedUp for Schedule:File module and total simulation time when comparing this branch to develop is shown in Fig.2. Where percentage speed-up is calculated as (t_develop - t_branch)*100/t_branch.

Fig 2.

We observe that the percentage speed-up is greater as the size and number of schedules to be parsed become greater.
We also observe that the improvements in run-time don't affect the IDFs with a smaller number of schedules very much. This is because of two reasons:
i. The percentage of time spent on schedule manager is low, compared to the rest of the simulation as seen in Fig 3. Therefore, even huge improvements in the Schedule:Manager can only improve the total simulation run time by so much.
ii. The improvement in schedule manager simulation is also low since the file open-close operations are very less.

Fig 3.

Time measured using the Chrono library between points a and b.
The test IDFs (added in this commit ) were created by adding Schedule:File objects that used information from the SolarShadingTest_Shading_Data.csv. The schedule manager will process any Schedule:File object that is present in the IDF irrespective of whether it is used or not. This 'problem' was exploited to create this performance chart.

Going Forward:

1. This refactor seems to show a great improvement for bigger schedules. The File:Schedule:Shading module will greatly benefit from this methodology, mainly because the HybridZoneModel_shadow113.idf references the whole SolarShadingTest_Shading_Data.csv, and the average run time decreased from 17s to 7s on my local machine.
2. My local machine ran through the smaller schedule test files quickly. I think someone with a slower machine might find improvements (by shaving off a few seconds) in the smaller IDFs also. (Referencing the ERR files from CI here).
3. There were slight variations in the total simulation time for similar runs. The data shown in the graphs are averages of three runs, the complete data file is linked below. An interesting observation was that the simulation time slightly increased sometimes even when the time to run schedule manager was decreased. I thought about using callgrind here but it will not measure performance improvements from reducing file operations.
4. I don't understand why I am getting the unused variable warning.
5. There is an issue if someone pointed multiple schedule objects to the same CSV column. I set up a fatal error for this scenario. This would have worked with the code in the current develop.

Data Used For Graphs - Link

TO DO

• Implement many to one mapping support.

NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE

Pull Request Author

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

• Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
• Label the PR with at least one of: Defect, Refactoring, NewFeature, Performance, and/or DoNoPublish
• Pull requests that impact EnergyPlus code must also include unit tests to cover enhancement or defect repair
• Author should provide a "walkthrough" of relevant code changes using a GitHub code review comment process
• If any diffs are expected, author must demonstrate they are justified using plots and descriptions
• If changes fix a defect, the fix should be demonstrated in plots and descriptions
• If any defect files are updated to a more recent version, upload new versions here or on DevSupport
• If IDD requires transition, transition source, rules, ExpandObjects, and IDFs must be updated, and add IDDChange label
• If structural output changes, add to output rules file and add OutputChange label
• If adding/removing any LaTeX docs or figures, update that document's CMakeLists file dependencies

Reviewer

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• If branch is behind develop, merge develop and build locally to check for side effects of the merge
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified
• Check that performance is not impacted (CI Linux results include performance check)
• Run Unit Test(s) locally
• Check any new function arguments for performance impacts
• Verify IDF naming conventions and styles, memos and notes and defaults
• If new idf included, locally check the err file and other outputs

[jmythms]

The main concept is that the processing moves from a Schedule:File object perspective to a CSV file perspective since we need to decrease file open and close operations as much as possible.

[jmythms]

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.

[jmythms]

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

[jmythms]

I wrapped the getObjectItem function and all the variable assignments into this PreProcessIDF function. This will populate the allIdfSchedData vector. We can access every Schedule:File object data from here, since these have been saved as elements of the allIdfSchedData vector. The main module also looks cleaner.

[jmythms]

This function runs through the IDF data and saves all the CSV file names to the std::set setOfFilenames. std::set makes sure that the list only has unique members in it.

[jmythms]

If we do not find the file name in the set, add it to the set...

[jmythms]

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

[mitchute]

All it takes is 💵 💵 💵 .

Maybe it has to do with the fact that you're returning a std::string but the new function has a std::string_view return type?

[mitchute]

I'm curious why this is necessary if you're not actually initializing to a value?

[jmythms]

It would automatically initialize it to 0 if I used the {}. I will change it to {0} for clarity 👍🏽

[mitchute]

I see. Thanks for clarifying.

[jmythms]

All it takes is .

Maybe it has to do with the fact that you're returning a std::string but the new function has a std::string_view return type?

I will check it out.

[Myoldmopar]

Yup, that's it.

https://godbolt.org/z/TqfqWWoTz

@amirroth has requested that we make a pass through the code to add string_view to places which may involve adding specialized functions for string_view as an alternate to the existing Objexx methods. I believe this is one such case. Perhaps for the sake of this PR, we can avoid going to string_view right now but use this as a first example use to dig in to string_view. Or maybe in this case, string_view is not actually the right thing to use and it really should be a string itself here. I've still got to learn a little bit more to be able to answer that without looking at the documentation at the same time.

[jmythms]

Thanks for figuring it out so quickly. I will take out the string_view stuff for this PR 👍🏽 I did learn so much from it though!

[Myoldmopar]

@jmythms what's the status of this now? I see it is still a draft PR, so I won't dig in much, but at a minimum, CI is complaining about some unused variables. Let me know if you need anything from me here. This'll be a good one though!

[nrel-bot-3]

@jmythms @lgentile it has been 28 days since this pull request was last updated.

[nrel-bot]

@jmythms @lgentile it has been 28 days since this pull request was last updated.

[nrel-bot]

@jmythms @lgentile it has been 29 days since this pull request was last updated.

[jmythms]

Closing this PR:

The problems intended to be fixed in this PR have already been addressed in PR#8996 and is part of Develop.

• Reducing repeating the file open-close operations have been fixed.
• Parsing CSV files line by line have also been implemented.

Running the HybridZoneModel_shadow113.idf file through the Clion Profiler, we see the CPU time spent on ProcessScheduleInput() has reduced from
73.98% - 7415d82 (Develop at the time of last update of this PR).
19% - This PR.
10% - b448a82 (Current Develop, which also includes additional improvements to ProcessNumber function, etc).

As the identified problems have already been solved, and reading the file is just taking ~10% CPU time of the simulation process for a 'large' CSV file, I propose to close this PR.

The small advantage I see from continuing this work is reducing the dependency of E+ on third-party libraries like nlohmann.
Since nlohmann is already used in the input processor since 2019, and the nlohmann license grants permission to modify it if necessary, I think it is too minor an advantage to pursue.

Open to discussion, if necessary.

@Myoldmopar