Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days #791

antonszilasi · 2025-02-16T01:35:20Z

WIP to address issue 153 and sketching out some ideas

github-actions · 2025-02-16T01:35:33Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

src/openstudio_lib/LocationTabView.cpp

macumber · 2025-02-16T22:25:59Z

Thanks @antonszilasi! I think the first thing to do is to create a mockup of the UI that we want to make, the unmethours post suggested this from Trace:

Once we have the mockup, we can create the widget to match it.

macumber · 2025-02-16T23:22:37Z

What about something like below:

We would look through the file and identify all the design day types and levels, then group them in a table. Each row would allow selecting one radio button. Each column would have a select all button to select all in that column

antonpsd · 2025-02-21T05:36:17Z

jmarrec · 2025-02-27T15:46:44Z

Marking the PR as non-draft because I want a CI run on it. @antonszilasi I fixed up the clang format and the cppcheck thingy quickly.

src/openstudio_lib/LocationTabView.cpp

jmarrec · 2025-02-27T15:53:07Z

src/openstudio_lib/LocationTabView.cpp

+
+std::vector<model::DesignDay> filterDesignDays(const std::vector<model::DesignDay>& designDays, const std::string& dayType,
+                                               const std::string& percentage, const std::string& humidityConditionType = "") {
+  std::vector<model::DesignDay> filteredDesignDays;


probably reserve first.

I think you could just do it in place instead of copying around though.

@jmarrec

do you mean something like this?

filteredDesignDays.reserve(designDays.size());

@jmarrec
done here b76aeff

src/openstudio_lib/LocationTabView.cpp

jmarrec · 2025-02-27T16:04:18Z

src/openstudio_lib/LocationTabView.cpp

+    dialog.accept();
+  });
+
+  // Populate table for Heating and Cooling


I'd expect a check to not include the checkboxes if the design days aren't in the DDY file to begin with.

jmarrec · 2025-02-27T16:05:51Z

src/openstudio_lib/LocationTabView.cpp

+      connect(checkBox, &QCheckBox::toggled, [=, &designDaysToInsert](bool checked) {
+        if (checked) {
+          std::string dayType = (row == 0) ? "WinterDesignDay" : "SummerDesignDay";
+          std::vector<model::DesignDay> filteredDays = filterDesignDays(allDesignDays, dayType, percentages[col]);
+          designDaysToInsert.insert(designDaysToInsert.end(), filteredDays.begin(), filteredDays.end());
+        } else {
+          // Remove unchecked design days
+          std::string dayType = (row == 0) ? "WinterDesignDay" : "SummerDesignDay";
+          std::vector<model::DesignDay> filteredDays = filterDesignDays(allDesignDays, dayType, percentages[col]);
+          for (const auto& day : filteredDays) {
+            auto it = std::find(designDaysToInsert.begin(), designDaysToInsert.end(), day);
+            if (it != designDaysToInsert.end()) {
+              designDaysToInsert.erase(it);
+            }
+          }
+        }
+        okButton->setEnabled(!designDaysToInsert.empty());
+      });
+    }
+  }


You could avoid constantly refiltering, delaying the filtering until the "Ok" button is clicked.

jmarrec · 2025-02-27T16:27:35Z

What about something like below:

We would look through the file and identify all the design day types and levels, then group them in a table. Each row would allow selecting one radio button. Each column would have a select all button to select all in that column

I like that a lot.

jmarrec · 2025-02-27T16:49:37Z

I started a test bed here in case it's useful: https://godbolt.org/z/rxern817h

antonszilasi · 2025-02-28T03:13:26Z

I have read the CLA Document and I hereby sign the CLA

Importing design days appears to be working

antonszilasi · 2025-02-28T05:04:57Z

src/openstudio_lib/LocationTabView.cpp

+    { 
+      return false;
+    }
+


@jmarrec somehow your other comments were lost but I addressed two of them in

78291c1

and a5263ba

antonszilasi · 2025-03-01T06:05:48Z

Importing ddy which is missing 0.4% design days
https://github.com/user-attachments/assets/d654ae29-e2cd-4267-ae15-9d34aedc9ce6

src/model_editor/InspectorGadget.cpp

…levels, add a gtest

… instead, attempt to fix the Mac CI issue. Really want NREL/OpenStudio#5229 to be fixed as the root cause

src/openstudio_lib/LocationTabView.hpp

macumber · 2025-03-02T19:13:05Z

I'm trying to be nice and include GIFs :-) You can see there is some existing ugliness in the DesignDayGridView when deleting DDYs on the Location tab (it doesn't fully delete/hide the row), we could try to clean that up in this PR:

jmarrec · 2025-03-04T16:34:42Z

I started a test bed here in case it's useful: https://godbolt.org/z/rxern817h

I think your interface is nice as is @macumber. I was originally thinking we've do something fancier/more explicitly per this link I started, but I don't know if it's worth overdoing it.

    std::vector<std::string> heatingPercentages = {"99.6%", "99%"};
    std::vector<std::string> coolingPercentages = {"2%", "1%", ".4%"};

    std::vector<DesignDayInfo> designDayTypes = {
        // Heating
        {.day_type = DesignDayInfo::DayType::Heating, .lookup_string = "DB", .name = "Heating", .tooltip = "Sizing heating equipment"},
        {.day_type = DesignDayInfo::DayType::Heating, .lookup_string = "WS=>MCDB", .name = "Heating Windy", .tooltip = "Used for sizing based on infiltration"},
        {.day_type = DesignDayInfo::DayType::Heating, .lookup_string = "DP=>MCDB", .name = "Heating Humid", .tooltip = "Used for sizing dehumidification systems"},

        // Cooling
        {.day_type = DesignDayInfo::DayType::Cooling, .lookup_string = "DB=>MWB", .name = "Cooling", .tooltip = "Sizing Chillers or air-conditioning units"},
        {.day_type = DesignDayInfo::DayType::Cooling, .lookup_string = "WB=>MDB", .name = "Cooling Latent", .tooltip = "Used to size cooling towers, evap-coolers, and fresh-air ventilation systems"},
        {.day_type = DesignDayInfo::DayType::Cooling, .lookup_string = "DP=>MDB", .name = "Cooling Dehum", .tooltip = "Used for sizing dehumidification systems"},
        {.day_type = DesignDayInfo::DayType::Cooling, .lookup_string = "Enth=>MDB", .name = "Cooling Enthalpy", .tooltip = "Used for sizing based on infiltration and/or ventilation"},

    };

It would look kinda like your mockup but with extra rows for the cooling ones and tooltips

…objects are deleted

src/shared_gui_components/OSCellWrapper.cpp

Copilot

Pull Request Overview

This PR aims to address issue 153 by allowing users to selectively import design days with specific percentages (99%, 99.6%, 0.4%, 1%, and 2%) to better refine annual design day selections.

Added grid cell selection signal emissions in OSObjectSelector.
Introduced style updating and header/visibility properties in OSCellWrapper.
Implemented a dialog in LocationTabView for selecting design days and updated associated test cases.

Reviewed Changes

Copilot reviewed 11 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/shared_gui_components/OSObjectSelector.cpp	Emits grid row selection changes after updating cell info.
src/shared_gui_components/OSCellWrapper.hpp & OSCellWrapper.cpp	Adds updateStyle functionality and new member variables to control style.
src/openstudio_lib/LocationTabView.hpp & LocationTabView.cpp	Introduces design day selection dialog, SortableDesignDay helper, and modifies design day insertion/removal logic.
src/openstudio_lib/test/LocationTab_GTest.cpp	Updates test cases for SortableDesignDay behavior.
.github/workflows/app_build.yml	Updates Conan install flags for improved compatibility with recent Clang.

Files not reviewed (12)

src/openstudio_lib/CMakeLists.txt: Language not supported
translations/OpenStudioApp_ar.ts: Language not supported
translations/OpenStudioApp_ca.ts: Language not supported
translations/OpenStudioApp_de.ts: Language not supported
translations/OpenStudioApp_el.ts: Language not supported
translations/OpenStudioApp_es.ts: Language not supported
translations/OpenStudioApp_fa.ts: Language not supported
translations/OpenStudioApp_fr.ts: Language not supported
translations/OpenStudioApp_he.ts: Language not supported
translations/OpenStudioApp_hi.ts: Language not supported
translations/OpenStudioApp_it.ts: Language not supported
translations/OpenStudioApp_ja.ts: Language not supported

Comments suppressed due to low confidence (1)

src/shared_gui_components/OSCellWrapper.cpp:585

[nitpick] Consider adding inline comments to clarify the bit ordering in the 'style' bitset to help future maintainers understand how the style string is constructed.

std::bitset<2> style;

Copilot · 2025-04-25T00:44:04Z

src/openstudio_lib/LocationTabView.cpp

+  QPushButton* cancelButton = new QPushButton(tr("Cancel"), &dialog);
+  connect(cancelButton, &QPushButton::clicked, &dialog, &QDialog::reject);
+
+  // import all imports everythig


Typo in comment: 'everythig' should be corrected to 'everything'.

Suggested change

// import all imports everythig

// import all imports everything

antonszilasi changed the title ~~Dev/anton/153~~ WIP to address issue 153 and sketching out some ideas Feb 16, 2025

macumber reviewed Feb 16, 2025

View reviewed changes

src/openstudio_lib/LocationTabView.cpp Outdated Show resolved Hide resolved

jmarrec marked this pull request as ready for review February 27, 2025 15:45

jmarrec added Enhancement Request New feature or request component - UI labels Feb 27, 2025

jmarrec assigned antonszilasi Feb 27, 2025

jmarrec reviewed Feb 27, 2025

View reviewed changes

github-actions bot added a commit that referenced this pull request Feb 28, 2025

@antonszilasi has signed the CLA in #791

821c858

macumber and others added 14 commits February 27, 2025 22:51

Clang format

91e8b00

Checkpoint select imports working

9bde301

Checkpoint appears to be working

068e8c3

Checkpoint

ffc742d

Checkpoint looking better

de4cd9a

this is looking good

67aa493

Checkpoint only considering annual design days now

5a781d2

Good checkpoint,

5a7cc1e

Importing design days appears to be working

Highlight things

f0997a8

Checkpoint, remove widgets before they are added

e192716

Checkpoint clean up and appears to be working

2d7eb11

Checkpoint this is much cleaner

33a0e60

Checkpoint make the interface more like what Dan Macumber suggested

3c594ab

Even better

65644e8

antonszilasi added 2 commits February 27, 2025 22:51

Just use boost::to_lower_copy

78291c1

use QString contains and use openstudio::isEqual

a5263ba

antonszilasi force-pushed the dev/anton/153 branch from 3e269fe to a5263ba Compare February 28, 2025 04:58

antonszilasi commented Feb 28, 2025

View reviewed changes

antonszilasi added 3 commits February 28, 2025 22:11

Rever space for DesignDaysToInsert

b76aeff

Avoid refiltering and delaying filtering until Ok button is clicked

b620b98

Only add check box if the design days exist in ddy

73a9060

antonszilasi added 2 commits March 1, 2025 00:08

clag format

99a64c7

this is not needed

d763728

antonszilasi changed the title ~~WIP to address issue 153 and sketching out some ideas~~ Attempting to address 153 Mar 1, 2025

antonszilasi commented Mar 1, 2025

View reviewed changes

src/model_editor/InspectorGadget.cpp Show resolved Hide resolved

antonszilasi changed the title ~~Attempting to address 153~~ Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days Mar 1, 2025

antonszilasi and others added 6 commits March 1, 2025 14:35

Adding some documentation

1e4af8d

Update LocationTabView.cpp

d7d13af

Refactor code to use a helper class to find the design day types and …

10a08ee

…levels, add a gtest

Remove unintended change

5b73370

Fix cppcheck

a477eb4

Avoid copying QVector around by adding design day key to radio button…

ee66ca9

… instead, attempt to fix the Mac CI issue. Really want NREL/OpenStudio#5229 to be fixed as the root cause

macumber reviewed Mar 2, 2025

View reviewed changes

src/openstudio_lib/LocationTabView.hpp Show resolved Hide resolved

macumber added 2 commits March 8, 2025 11:03

Hide borders on cells when deleted, update select all check box when …

fc925bf

…objects are deleted

Clang format

7dbb1f0

macumber reviewed Mar 8, 2025

View reviewed changes

src/shared_gui_components/OSCellWrapper.cpp Show resolved Hide resolved

macumber added 3 commits March 8, 2025 20:36

Fixing bug in style which prevented lines from being drawn

1c153d9

Update all checkbox when adding new objects too

949b705

Clang format

4b70fb4

macumber requested a review from Copilot April 25, 2025 00:43

Copilot AI reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days #791

Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days #791

antonszilasi commented Feb 16, 2025 •

edited

Loading

github-actions bot commented Feb 16, 2025 •

edited

Loading

macumber commented Feb 16, 2025

macumber commented Feb 16, 2025 •

edited

Loading

antonpsd commented Feb 21, 2025

jmarrec commented Feb 27, 2025

jmarrec Feb 27, 2025

antonszilasi Feb 28, 2025 •

edited

Loading

jmarrec Feb 28, 2025

antonszilasi Mar 1, 2025 •

edited

Loading

jmarrec Feb 27, 2025

antonszilasi Mar 1, 2025

jmarrec Feb 27, 2025

antonszilasi Mar 1, 2025

jmarrec commented Feb 27, 2025

jmarrec commented Feb 27, 2025

antonszilasi commented Feb 28, 2025

antonszilasi Feb 28, 2025

antonszilasi commented Mar 1, 2025 •

edited

Loading

macumber commented Mar 2, 2025 •

edited

Loading

jmarrec commented Mar 4, 2025 •

edited

Loading

Copilot AI left a comment

Copilot AI Apr 25, 2025

	// import all imports everythig
	// import all imports everything

Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days #791

Are you sure you want to change the base?

Attempting to address 153, by allowing user to specifically import 99%,99.6%, 0.4%,1% and 2% annual design days #791

Conversation

antonszilasi commented Feb 16, 2025 • edited Loading

github-actions bot commented Feb 16, 2025 • edited Loading

macumber commented Feb 16, 2025

macumber commented Feb 16, 2025 • edited Loading

antonpsd commented Feb 21, 2025

jmarrec commented Feb 27, 2025

jmarrec Feb 27, 2025

Choose a reason for hiding this comment

antonszilasi Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

jmarrec Feb 28, 2025

Choose a reason for hiding this comment

antonszilasi Mar 1, 2025 • edited Loading

Choose a reason for hiding this comment

jmarrec Feb 27, 2025

Choose a reason for hiding this comment

antonszilasi Mar 1, 2025

Choose a reason for hiding this comment

jmarrec Feb 27, 2025

Choose a reason for hiding this comment

antonszilasi Mar 1, 2025

Choose a reason for hiding this comment

jmarrec commented Feb 27, 2025

jmarrec commented Feb 27, 2025

antonszilasi commented Feb 28, 2025

antonszilasi Feb 28, 2025

Choose a reason for hiding this comment

antonszilasi commented Mar 1, 2025 • edited Loading

macumber commented Mar 2, 2025 • edited Loading

jmarrec commented Mar 4, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

antonszilasi commented Feb 16, 2025 •

edited

Loading

github-actions bot commented Feb 16, 2025 •

edited

Loading

macumber commented Feb 16, 2025 •

edited

Loading

antonszilasi Feb 28, 2025 •

edited

Loading

antonszilasi Mar 1, 2025 •

edited

Loading

antonszilasi commented Mar 1, 2025 •

edited

Loading

macumber commented Mar 2, 2025 •

edited

Loading

jmarrec commented Mar 4, 2025 •

edited

Loading