Fix ASHP min allowable size enforcement

[zolanaj]

Addresses #448

[Bill-Becker]

Hey Alex, this is more of a question from the original PR than this fix, but it looks like there's a couple of places where we have default values hard-coded in various places which seem redundant to defaults which are either stored in the ashp_defaults.json file or in other locations in /core/ashp.jl:

This peak_load_thermal_factor looks like it's meant to be the same value as the min_allowable_peak_capacity_fraction:

REopt.jl/src/core/ashp.jl

Line 498 in 452a717

peak_load_thermal_factor::Real = 0.5

And this default for the value of back_up_temp_threshold_degF for this function:

REopt.jl/src/core/ashp.jl

Line 447 in 452a717

back_up_temp_threshold_degF = 10.0

Not a big deal, but ideally we would only have to make changes to the default values in one place in the future. If you want to punt on updating for now, maybe we could at least make a comment for the first time these parameters are assigned the default value that if the default changes, you also need to update in these other locations.

[Bill-Becker]

Approved with a suggestion about linking defaults for a couple of parameters.

[zolanaj]

Hey Alex, this is more of a question from the original PR than this fix, but it looks like there's a couple of places where we have default values hard-coded in various places which seem redundant to defaults which are either stored in the ashp_defaults.json file or in other locations in /core/ashp.jl:

This peak_load_thermal_factor looks like it's meant to be the same value as the min_allowable_peak_capacity_fraction:

REopt.jl/src/core/ashp.jl

Line 498 in 452a717

peak_load_thermal_factor::Real = 0.5

And this default for the value of back_up_temp_threshold_degF for this function:

REopt.jl/src/core/ashp.jl

Line 447 in 452a717

back_up_temp_threshold_degF = 10.0

Not a big deal, but ideally we would only have to make changes to the default values in one place in the future. If you want to punt on updating for now, maybe we could at least make a comment for the first time these parameters are assigned the default value that if the default changes, you also need to update in these other locations.

Thanks @Bill-Becker! I agree, we should not have these defaults in place within the function definition - they should be in one place. I've pushed an update that should remove then and a fix to some tests that were previously incomplete. Neither function exports to the API so it shouldn't be a breaking change. I'll merge once I've confirmed tests pass.