From c5f17910dca5023d1d8fa7a46d8a4eef69e5dccb Mon Sep 17 00:00:00 2001 From: Lerond Date: Fri, 16 May 2025 15:24:30 -0700 Subject: [PATCH 1/5] Recalculate outlet water temperature. --- src/EnergyPlus/WaterCoils.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/EnergyPlus/WaterCoils.cc b/src/EnergyPlus/WaterCoils.cc index ff3cd4e8a9d..6e15af444af 100644 --- a/src/EnergyPlus/WaterCoils.cc +++ b/src/EnergyPlus/WaterCoils.cc @@ -2835,6 +2835,7 @@ void CalcSimpleHeatingCoil(EnergyPlusData &state, if (fanOp == HVAC::FanOp::Cycling) { HeatingCoilLoad *= PartLoadRatio; + TempWaterOut = TempWaterIn - HeatingCoilLoad / CapacitanceWater; } // Set the outlet conditions From fa2341477797c6f875bc0aec267f5a2a8681d473 Mon Sep 17 00:00:00 2001 From: Lerond Date: Fri, 16 May 2025 17:09:28 -0700 Subject: [PATCH 2/5] Catch div by 0. --- src/EnergyPlus/WaterCoils.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/EnergyPlus/WaterCoils.cc b/src/EnergyPlus/WaterCoils.cc index 6e15af444af..1ed6fd2ae51 100644 --- a/src/EnergyPlus/WaterCoils.cc +++ b/src/EnergyPlus/WaterCoils.cc @@ -2835,7 +2835,9 @@ void CalcSimpleHeatingCoil(EnergyPlusData &state, if (fanOp == HVAC::FanOp::Cycling) { HeatingCoilLoad *= PartLoadRatio; - TempWaterOut = TempWaterIn - HeatingCoilLoad / CapacitanceWater; + if (CapacitanceWater > 0) { + TempWaterOut = TempWaterIn - HeatingCoilLoad / CapacitanceWater; + } } // Set the outlet conditions From eb50425a739e15aed573215a4c9acfc065db2c3d Mon Sep 17 00:00:00 2001 From: lymereJ Date: Sun, 25 May 2025 23:27:28 -0700 Subject: [PATCH 3/5] Better fix. --- src/EnergyPlus/WaterCoils.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/EnergyPlus/WaterCoils.cc b/src/EnergyPlus/WaterCoils.cc index 8689cfd894b..c01dba5d4a9 100644 --- a/src/EnergyPlus/WaterCoils.cc +++ b/src/EnergyPlus/WaterCoils.cc @@ -2903,7 +2903,8 @@ void CalcSimpleHeatingCoil(EnergyPlusData &state, if (fanOp == HVAC::FanOp::Cycling) { HeatingCoilLoad *= PartLoadRatio; - if (CapacitanceWater > 0) { + if (CapacitanceWater > 0 && + state.dataPlnt->PlantLoop(waterCoil.WaterPlantLoc.loopNum).LoopSide(DataPlant::LoopSideLocation::Supply).FlowRequest > 0) { TempWaterOut = TempWaterIn - HeatingCoilLoad / CapacitanceWater; } } From 63976381ab396ad606984353e517cb68d902ca2f Mon Sep 17 00:00:00 2001 From: Edwin Lee Date: Thu, 12 Jun 2025 22:24:08 -0500 Subject: [PATCH 4/5] Try a regression update on a branch with diffs --- .github/workflows/test_pull_requests.yml | 7 + scripts/dev/gha_regressions.py | 282 ++++++++++++++++++++++- 2 files changed, 288 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test_pull_requests.yml b/.github/workflows/test_pull_requests.yml index 05f549c93a4..bc2becc71c8 100644 --- a/.github/workflows/test_pull_requests.yml +++ b/.github/workflows/test_pull_requests.yml @@ -184,6 +184,13 @@ jobs: name: "regressions-${{ matrix.os }}" path: "${{ github.workspace }}/regressions" + - uses: actions/upload-artifact@v4 + id: upload_regression_plotter + if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' # only run this if regressions were encountered "failed" + with: + name: "regressions-plotter-${{ matrix.os }}" + path: "${{ github.workspace }}/regression_plotter.html" + - name: Generate Regression Summary GitHub Script if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' run: > diff --git a/scripts/dev/gha_regressions.py b/scripts/dev/gha_regressions.py index cf62bb9ec0c..9612706552a 100644 --- a/scripts/dev/gha_regressions.py +++ b/scripts/dev/gha_regressions.py @@ -54,6 +54,7 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. from collections import defaultdict +import csv from datetime import datetime, UTC import json from shutil import copy @@ -459,11 +460,225 @@ def generate_markdown_summary(self, bundle_root: Path): """ (bundle_root / 'summary.md').write_text(content) + @staticmethod + def read_csv_to_columns(file_path: Path) -> dict[str, list[str | float]]: + columns = {} + with open(file_path, 'r', newline='') as file: + reader = csv.reader(file) + header = next(reader) # Read header row + for i, col_name in enumerate(header): + columns[col_name.strip()] = [] + for row in reader: + for i, value in enumerate(row): + value = value.strip() + variable = header[i].strip() + try: + v = float(value) + columns[variable].append(v) + except ValueError: # just take the string timestamp + columns[variable].append(value) + return columns + + @staticmethod + def generate_regression_plotter(bundle_root: Path, metadata_object: dict, results_object: dict, limit: bool): + metadata_string = json.dumps(metadata_object, indent=4) + results_string = json.dumps(results_object, indent=4) + diagnostics_string = "" + limit_string = "" if not limit else r"""""" + content = r""" + + + + EnergyPlus Regression Viewer + + + + + +
+

EnergyPlus Regression Quick Plotter

+
+ + +
+
+ + +
+ + + {{LIMIT}} +
+
+
+ + + + """ + patches = { + '{{METADATA}}': metadata_string, + '{{RESULTS}}': results_string, + '{{LIMIT}}': limit_string + } + for key, value in patches.items(): + content = content.replace(key, value) + (bundle_root / 'regression_plotter.html').write_text(content) + def check_all_regressions(self, base_testfiles: Path, mod_testfiles: Path, bundle_root: Path) -> bool: any_diffs = False bundle_root.mkdir(exist_ok=True) entries = sorted(base_testfiles.iterdir()) backtrace_shown = False + + # temporarily hardcoding metadata stuff + baseline_name = "baseline" + branch_name = "branch" + branch_sha = "abc123de" + metadata_object = { + "baseline": baseline_name, "branch": branch_name, "branchSha": branch_sha + } + results_object = {} + hit_max_limit = False for entry_num, baseline in enumerate(entries): if not baseline.is_dir(): continue @@ -495,7 +710,66 @@ def check_all_regressions(self, base_testfiles: Path, mod_testfiles: Path, bundl contents = potential_diff_file.read_text() wrapped_contents = self.single_diff_html(contents) diff_file_with_html.write_text(wrapped_contents) - index_contents_this_file += self.regression_row_in_single_test_case_html(potential_diff_file.name) + if potential_diff_file.name == "eplusout.csv.absdiff.csv" and not hit_max_limit: + # get data from the csvs, for now just all columns + base_csv = potential_diff_file.parent / "eplusout.csv" + base_column_data = self.read_csv_to_columns(base_csv) + base_timestamps = base_column_data['Date/Time'] + mod_csv = mod_testfiles / baseline.name / "eplusout.csv" + mod_column_data = self.read_csv_to_columns(mod_csv) + mod_timestamps = mod_column_data['Date/Time'] + # but then downselect only the columns that math-diff provided in the summary + abs_diff_columns = self.read_csv_to_columns(potential_diff_file) + variables_with_diffs = abs_diff_columns.keys() + # Alright so here's the trick. If we can tell for sure that these results are + # just two design days only, split evenly, then we will split it into two + # plots. If not, we'll just do one big one + # at this point, we know the time series match between base and branch + do_multi_plot = False + mid_point = 0 + base_times = [base_timestamps] + mod_times = [mod_timestamps] + if len(base_timestamps) % 2 == 0: + mid_point = len(base_timestamps) // 2 + one = base_timestamps[:mid_point] + one_prefix = one[0][0:6] + one_all_same_prefix = all([x.startswith(one_prefix) for x in one]) + two = base_timestamps[mid_point:] + two_prefix = two[0][0:6] + two_all_same_prefix = all([x.startswith(two_prefix) for x in two]) + if one_all_same_prefix and two_all_same_prefix: + do_multi_plot = True + base_times = [one, two] + mod_times = [one, two] + results_object[baseline.name] = { + "timestamps": {"develop": base_times, "branch": mod_times} + } + for v in variables_with_diffs: + if v == 'Date/Time': + continue + base_data = [base_column_data[v]] + mod_data = [mod_column_data[v]] + if do_multi_plot: + plot_one_base_data = base_column_data[v][:mid_point] + plot_one_mod_data = mod_column_data[v][:mid_point] + plot_two_base_data = base_column_data[v][mid_point:] + plot_two_mod_data = mod_column_data[v][mid_point:] + base_data = [plot_one_base_data, plot_two_base_data] + mod_data = [plot_one_mod_data, plot_two_mod_data] + # temporarily hardcode the units and quantity + results_object[baseline.name][v] = { + "develop": base_data, + "branch": mod_data + } + # check if we are at the max limit to avoid making the page way too heavy to load + max_size_megabytes = 75 # 75 MB is probably a good cutoff + max_size_bytes = max_size_megabytes * 1024 * 1024 + estimated_size = len(json.dumps(results_object).encode('utf-8')) + if estimated_size > max_size_bytes: + hit_max_limit = True + index_contents_this_file += self.regression_row_in_single_test_case_html( + potential_diff_file.name + ) index_file = target_dir_for_this_file_diffs / 'index.html' index_this_file = self.single_test_case_html(index_contents_this_file) index_file.write_text(index_this_file) @@ -526,6 +800,12 @@ def check_all_regressions(self, base_testfiles: Path, mod_testfiles: Path, bundl print(f"* Diffs by Type *:\n{json.dumps(self.diffs_by_type, indent=2, sort_keys=True)}\n") if any_diffs: self.generate_markdown_summary(bundle_root) + self.generate_regression_plotter( + bundle_root, metadata_object, results_object, hit_max_limit + ) + self.generate_regression_plotter( + bundle_root.parent, metadata_object, results_object, hit_max_limit + ) # print("::warning title=Regressions::Diffs Detected") return any_diffs From ac19cafdab178bdd860196dbff3826166999bcd7 Mon Sep 17 00:00:00 2001 From: Edwin Lee Date: Fri, 13 Jun 2025 16:16:48 -0500 Subject: [PATCH 5/5] Annotate the GHA workflow a little, break out html plotter template, add diff plot where possible --- .github/workflows/test_pull_requests.yml | 13 +- scripts/dev/gha_regressions.py | 189 ++---------------- scripts/dev/regression_plotter.in.html | 241 +++++++++++++++++++++++ 3 files changed, 262 insertions(+), 181 deletions(-) create mode 100644 scripts/dev/regression_plotter.in.html diff --git a/.github/workflows/test_pull_requests.yml b/.github/workflows/test_pull_requests.yml index bc2becc71c8..488ec93a477 100644 --- a/.github/workflows/test_pull_requests.yml +++ b/.github/workflows/test_pull_requests.yml @@ -170,6 +170,7 @@ jobs: if: always() && matrix.run_regressions && steps.branch_build.outcome != 'failure' # always run this step as long as we actually built run: pip install energyplus-regressions>=2.1.4 + # This will build out a ./regressions package as well as a ./regressions-plotter-{{matric.os}}.html report - name: Run Regressions if: always() && matrix.run_regressions && steps.branch_build.outcome != 'failure' # always run this step as long as we actually built id: regressions @@ -177,20 +178,23 @@ jobs: continue-on-error: true run: python ./branch/scripts/dev/gha_regressions.py ./baseline/build/testfiles ./branch/build/testfiles/ ./regressions - - uses: actions/upload-artifact@v4 + - name: Upload Regressions Full Bundle + uses: actions/upload-artifact@v4 id: upload_regressions if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' # only run this if regressions were encountered "failed" with: name: "regressions-${{ matrix.os }}" path: "${{ github.workspace }}/regressions" - - uses: actions/upload-artifact@v4 + - name: Upload Regressions Plotter HTML + uses: actions/upload-artifact@v4 id: upload_regression_plotter if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' # only run this if regressions were encountered "failed" with: name: "regressions-plotter-${{ matrix.os }}" path: "${{ github.workspace }}/regression_plotter.html" + # This executes a Python script that reads regressions results and produces a JavaScript file meant to run on GitHub - name: Generate Regression Summary GitHub Script if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' run: > @@ -202,7 +206,9 @@ jobs: ${{ github.run_id }} ${{ steps.upload_regressions.outputs.artifact-url }} - - uses: actions/github-script@v7 + # Run the generated JavaScript file which will post the regressions summary to the PR itself + - name: Post Regression Summary to PR + uses: actions/github-script@v7 if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' && github.event.pull_request.head.repo.full_name == 'NREL/EnergyPlus' with: script: | @@ -211,6 +217,7 @@ jobs: # - uses: lhotari/action-upterm@v1 + # Unfortunately I can't get forked repository runs to post onto the original PR, so this is needed to alert for regressions - name: Fail on Regressions from Forked Repository if: always() && matrix.run_regressions && steps.regressions.outcome == 'failure' && github.event.pull_request.head.repo.full_name != 'NREL/EnergyPlus' run: | diff --git a/scripts/dev/gha_regressions.py b/scripts/dev/gha_regressions.py index 9612706552a..75b0421d45d 100644 --- a/scripts/dev/gha_regressions.py +++ b/scripts/dev/gha_regressions.py @@ -83,7 +83,7 @@ def __init__(self): import energyplus_regressions self.threshold_file = str(Path(energyplus_regressions.__file__).parent / 'diffs' / 'math_diff.config') - def single_file_regressions(self, baseline: Path, modified: Path) -> [TestEntry, bool]: + def single_file_regressions(self, baseline: Path, modified: Path) -> tuple[TestEntry, bool]: idf = baseline.name self.num_idf_inspected += 1 @@ -481,184 +481,17 @@ def read_csv_to_columns(file_path: Path) -> dict[str, list[str | float]]: @staticmethod def generate_regression_plotter(bundle_root: Path, metadata_object: dict, results_object: dict, limit: bool): - metadata_string = json.dumps(metadata_object, indent=4) - results_string = json.dumps(results_object, indent=4) - diagnostics_string = "" - limit_string = "" if not limit else r"""""" - content = r""" - - - - EnergyPlus Regression Viewer - - - - - -
-

EnergyPlus Regression Quick Plotter

-
- - -
-
- - -
- - - {{LIMIT}} -
-
-
- - - - """ + metadata_string = "metadata = " + json.dumps(metadata_object, indent=4) + ';' + results_string = "results = " + json.dumps(results_object, indent=4) + ';' + limit_string = "" + if limit: + limit_string = r"""""" + template_file = Path(__file__).resolve().parent / 'regression_plotter.in.html' + content = template_file.read_text() patches = { - '{{METADATA}}': metadata_string, - '{{RESULTS}}': results_string, - '{{LIMIT}}': limit_string + '': metadata_string, + '': results_string, + '': limit_string } for key, value in patches.items(): content = content.replace(key, value) diff --git a/scripts/dev/regression_plotter.in.html b/scripts/dev/regression_plotter.in.html new file mode 100644 index 00000000000..337d681f9cd --- /dev/null +++ b/scripts/dev/regression_plotter.in.html @@ -0,0 +1,241 @@ + + + + EnergyPlus Regression Viewer + + + + + +
+

EnergyPlus Regression Quick Plotter

+
+ + +
+
+ + +
+ + + + +
+
+
+ + + \ No newline at end of file