-
Notifications
You must be signed in to change notification settings - Fork 4
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
Multi speed fan #109
Multi speed fan #109
Conversation
Cannot pass the new generate test. |
… tests, add organizational comments.
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.
@wanhanlong1130 - I pushed a commit with a fix for the unit test and additional changes. Here's a code walkthrough. Please, let me know if you have any further comments. This PR could be merged if you are okay with these changes.
@@ -147,6 +168,37 @@ def __init__( | |||
plf_f_plr.out_max = 1 | |||
self.set_of_curves.append(plf_f_plr) | |||
|
|||
def calc_fan_power(self, capacity_ratio): |
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.
New function that calculates multi-speed fan power.
ieer_two_spd = round(DX.calc_rated_eff(), 2) | ||
assert ieer_two_spd > ieer | ||
|
||
def test_multi_speed(self): |
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.
New unit test to test the new function.
# Two-speed fan unit | ||
DX = cp.UnitaryDirectExpansion( | ||
compressor_type="scroll", | ||
condenser_type="air", | ||
compressor_speed="constant", | ||
ref_cap_unit="si", | ||
ref_gross_cap=471000, | ||
full_eff=5.89, | ||
full_eff_unit="cop", | ||
part_eff_ref_std="ahri_340/360", | ||
model="simplified_bf", | ||
sim_engine="energyplus", | ||
set_of_curves=lib.get_set_of_curves_by_name("D208122216").curves, | ||
indoor_fan_speeds=2, | ||
) | ||
ieer_two_spd = round(DX.calc_rated_eff(), 2) | ||
assert ieer_two_spd > ieer |
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.
Test that a two-speed fan unit has a better IEER than a single speed fan one.
|
||
# Define the base curves to be use as the starting point in the generation process | ||
base_curves = cp.SetofCurves() | ||
base_curves.curves = dx_unit.set_of_curves | ||
base_curves.eqp = dx_unit | ||
|
||
# Generate the curves | ||
set_of_curves = dx_unit.generate_set_of_curves( | ||
base_curves=[lib.get_set_of_curves_by_name("D208122216")], | ||
base_curves=[base_curves], |
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.
Fix the previously failing unit test.
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.
em... I just wonder why lib.get_set_of_curves_by_name("D208122216") cannot work - the code can run, but seems the results have some problems.
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.
That is because get_set_of_curves_by_name()
returns a SetofCurves()
but doesn't set the eqp
properties. I've never liked that eqp
has to be set for a SetofCurves()
, perhaps we could look into making that change later.
Pull request here to check any format or other issues.
Hold approval for discussion.