-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
Minimum allowed coverage is Generated by 🐒 cobertura-action against 9b86fac |
@torstenglidden Can you give this changelog a review? You can download the full docs here. |
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.
@nmerket The changelog generally looks good to me, but with the following comments/questions:
-
There were two other changes we announced that I didn't see changelog entries for:
a) Low-e storm windows
b) Added HVAC enumeration -- high-efficiency/cold climate heat pumps -
In the HVAC section, in the linked description for Duct Leakage Measurements, the 2nd bulleted item in the seemed confusing to me...it seems to cut of short of saying what happens -- e.g.,
If the above element is not found, DuctLeakageMeasurement[DuctType="supply"]/DuctLeakage[TotalOrToOutside="to outside" and Units="CFM25"]/Value and DuctLeakageMeasurement[DuctType="return"]/DuctLeakage[TotalOrToOutside="to outside" and Units="CFM25"]/Value elements with the numeric values (Note that the sum of two elements will be used)
-- I'm guessing it should read something like "If the above element is not found, then DuctLeakageMeasurement[DuctType="supply"]/DuctLeakage[TotalOrToOutside="to outside" and Units="CFM25"]/Value and DuctLeakageMeasurement[DuctType="return"]/DuctLeakage[TotalOrToOutside="to outside" and Units="CFM25"]/Value elements with the numeric values will be used (Note that the sum of the two elements will be used)". Would that still be an accurate description (it seems clearer to me that way) or am I missing something else?
- In the Water Heating section, I think it would be helpful to add "gas" before both instances of "water heater", just to avoid confusion with regard to the higher EF for heat pump water heaters and the "maximum allowed".
Both of these were added as upgrades only, so they don't really affect the behavior this translator code. A user can specify a high efficiency heat pump already in the base building by indicating an appropriate HSPF and SEER. Same for Low-e storms using U-factor and SHGC, although I think we should discuss in our meeting today whether we should try to translate a low-e storm more for a base building from some elements in HPXML.
Good catch. I'll fix that.
This also applies to electric resistance storage and tankless water heaters as well, but you're correct that it should be clarified that it does not apply to heat pump water heaters. I'll make that change. |
@nmerket The changelog generally looks good to me, but with the following
comments/questions:
1. There were two other changes we announced that I didn't see changelog
entries for:
a) Low-e storm windows
b) Added HVAC enumeration -- high-efficiency/cold climate heat pumps
2. In the HVAC section, in the linked description for Duct Leakage
Measurements, the 2nd bulleted item in the seemed confusing to me...it
seems to cut of short of saying what happens -- e.g.,
-
If the above element is not found,
DuctLeakageMeasurement[DuctType="supply"]/DuctLeakage[TotalOrToOutside="to
outside" and Units="CFM25"]/Value and
DuctLeakageMeasurement[DuctType="return"]/DuctLeakage[TotalOrToOutside="to
outside" and Units="CFM25"]/Value elements with the numeric values
(Note that the sum of two elements will be used)
…-- I'm guessing it should read something like "*If the above element is not
found, then
DuctLeakageMeasurement[DuctType="supply"]/DuctLeakage[TotalOrToOutside="to
outside" and Units="CFM25"]/Value
and
DuctLeakageMeasurement[DuctType="return"]/DuctLeakage[TotalOrToOutside="to
outside" and Units="CFM25"]/Value
elements with the numeric values will be used (Note that the sum of the two
elements will be used)*". Would that still be an accurate description (it
seems clearer to me that way) or am I missing something else?
3. In the Water Heating section, I think it would be helpful to add "gas"
before both instances of "water heater", just to avoid confusion with
regard to the higher EF for heat pump water heaters and the "maximum
allowed".
On Thu, Apr 21, 2022 at 2:03 PM Noel Merket ***@***.***> wrote:
@torstenglidden <https://github.com/torstenglidden> Can you give this
changelog a review? You can download the full docs here
<https://github.com/NREL/hescore-hpxml/suites/6212329655/artifacts/219590898>
.
—
Reply to this email directly, view it on GitHub
<#198 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWOE44UOGUUWIRAHPJ64MLDVGG7B3ANCNFSM5UAMUFKQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Fixes #192
Pull Request Description
This updates the version to 7.0 in conjunction with the OpenStudio-HEScore v2.0 release.
Also includes a changelog describing the updates.
Checklist
PR Author: Check these when they're done. Not all may apply.
strikethroughand check any that do not apply.PR Reviewer: Verify each has been completed.
Code changes (must work)Test exercising your feature or bug fix. Check the coverage report in the build artifacts.