-
Notifications
You must be signed in to change notification settings - Fork 41
Remove ghp updates #519
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
base: develop
Are you sure you want to change the base?
Remove ghp updates #519
Conversation
This reverts commit 8eb7761.
This reverts commit 03bf61a.
This reverts commit 1f65aa5.
This reverts commit 344dd8f.
This reverts commit 9b9995b.
This reverts commit f2b9354.
This reverts commit 19429d8.
This reverts commit 2968682.
This reverts commit 67d9ff9.
This reverts commit 902e999.
This reverts commit 3c49a86.
This reverts commit 228ad0e.
This reverts commit f99997b.
This reverts commit 971b3e7.
…ling for hybrid GHP" This reverts commit b584149.
This reverts commit 69b7017.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reverts changes made in #459 that implemented hybrid GHP functionality. The main purpose is to remove the complex hybrid GHP sizing workflow and return to the simpler automatic GHP sizing approach.
Key changes include:
- Removed hybrid GHP test scenarios and test code
- Simplified GHP sizing logic by removing hybrid workflow iterations
- Removed hybrid-related fields from GHP struct and results
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
test/scenarios/ghp_inputs_hybrid.json | Removed hybrid GHP test scenario file |
test/runtests.jl | Removed hybrid GHP functionality tests |
src/results/heating_cooling_load.jl | Added whitespace formatting |
src/results/ghp.jl | Simplified GHP results by removing hybrid fields and rounding |
src/core/scenario.jl | Simplified GHP sizing logic by removing hybrid workflow |
src/core/ghp.jl | Removed hybrid-related fields from GHP struct |
CHANGELOG.md | Removed hybrid-related changelog entry |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
is_ghx_hybrid = true | ||
hybrid_sizing_flag = get(ghpghx_inputs, "hybrid_ghx_sizing_fraction", 0.6) | ||
elseif hybrid_ghx_sizing_method !== nothing | ||
else |
Copilot
AI
Oct 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This else
clause will match any non-nil hybrid_ghx_sizing_method
value other than 'Automatic' or 'Fractional', but the logic suggests it should only match when hybrid_ghx_sizing_method
is not nothing
. Change to elseif hybrid_ghx_sizing_method !== nothing
.
else | |
elseif hybrid_ghx_sizing_method !== nothing |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about this, can be dealt with when the GHP updates are fixed and reintroduced
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
and update docstrings
reverts the changes made in #459