-
Notifications
You must be signed in to change notification settings - Fork 461
Fix VRF Heat Pump Total Heating Rate not match the sum of coil heating rate (with piping loss) #10627
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
Open
yujiex
wants to merge
23
commits into
develop
Choose a base branch
from
fixDocVRFheatpumpHeatRate
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+74
−3
Open
Fix VRF Heat Pump Total Heating Rate not match the sum of coil heating rate (with piping loss) #10627
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
f5c5103
update doc: HP total heating rate = sum coil heating rate
03d9070
change FanSpdRatio calculation to use enthalpy difference
098e584
Fix non-matching HP heating rate and coil total heating rate
a75d23a
latex label add suffix, revert FanSpdRatioMin change
6379fdc
Doc update, HP heating rate is coil + piping loss
f306890
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
94e3c1f
change to directly use limited TU_HeatingLoad
ac4911c
revert test idf back to autosize heating cooling air flow TU1
c9fb07f
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
94fbca7
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
3ba669a
Revert "change to directly use limited TU_HeatingLoad"
303a572
Revert "Fix non-matching HP heating rate and coil total heating rate"
0ca1aa3
put back check on whether endpoints both positive or negative
fbcb90c
remov piping loss in coil heating calculation for now
bc5d7c9
Wrongly removed a line, add back
6161f8d
add back piping correction to heating coil heating rate
e1db7ed
revert back the FanSpdRatioMin bound as in develop
65f2c96
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
411d96a
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
4075b7f
revert changes in TotalTUHeatingCapacity,coil pipe correction
939896c
remove 100W lower bound on coil demand in FanSpdResidualHeat
0958045
update coil heating rate with latest HP capacity, piping correction
04a44d2
Merge remote-tracking branch 'origin/develop' into fixDocVRFheatpumpH…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Why is the parent writing to the child? The parent should call the child and the child should calculate it's own operation. Isn't the child heating coil TotalHeatingEnergyRate here already equal to AirMassFlow * (OutletAirEnthalpy - heatingCoil.InletAirEnthalpy)? i.e., wasn't TotalHeatingEnergyRate set in the child on the call above?
Uh oh!
There was an error while loading. Please reload this page.
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.
The coil TotalHeatingEnergyRate was set when the demand
QCoilReqwas calculated with the old piping loss (calculated with rated values) and OU capacity. The following shows the section in the PR description with details on this. 0.986 piping loss used the rated capacity of 7000W in the calculation. So the above is added to recalculate the coil heating rate with the updated piping loss.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.
When the parent changes the child's TotalHeatingEnergyRate, should the heating coil outlet air condition also change?
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.
oh probably that too. I'll add these to save the updated outlet values
Uh oh!
There was an error while loading. Please reload this page.
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.
And if the heating coil outlet condition changes, then doesn't the fan speed need to change to meet the same load? And if the fan speed changes doesn't the fan heat also change? This is a good example of why the parent shouldn't update child component performance. Couldn't you just send the piping loss adjusted QCoilReq over to the heating coil?
Uh oh!
There was an error while loading. Please reload this page.
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.
I don't see piping losses used in LimitCoilCapacity to calculate the limit, if piping losses were used there would that correct the problem?
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.
When the TU capacity is limited the OU will be running at full load. I think you are saying you don't know the operating full load capacity yet. I think that's OK and the next iteration will include the correct piping losses that were calculated at the end of the previous iteration. I think there will be, within a time step's iteration sequence, a FirstHVACIteration = true, FirstHVACIteration = false, and then a final FirstHVACIteration = false (although I am not positive about this last one happening all the time). So the piping losses should be correct on the second iteration and the final iteration should have the correct answer? I do still think PipingLosses needs to be included in LimitCoilCapacity.
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.
I just checked the calls into LimitCoilCapacity and what is passed in as TotalCapacity is already corrected for piping losses. That makes me wonder what is actually happening for this issue.
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.
Does this issue only happen in heating mode?
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.
it did correct for piping loss, but the piping correction factor was computed using the rated capacity, not the actual capacity, so that piping correction is also not right at the time of limitTUCapacity
This issue also happens for cooling mode
