Skip to content
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

Consolidate and remove duplicated 90.1 PRM methods #1441

Open
mdahlhausen opened this issue Mar 14, 2023 · 2 comments
Open

Consolidate and remove duplicated 90.1 PRM methods #1441

mdahlhausen opened this issue Mar 14, 2023 · 2 comments
Assignees
Labels
AppendixG Methods to enable the Appendix G model workflow Developer Issue

Comments

@mdahlhausen
Copy link
Collaborator

Going through the code I noticed that several of the ASHRAE 90.1 PRM methods are functionally identical duplicates of methods already in standards.

For example, the hot water boiler methods boiler_hot_water_find_search_criteria and boiler_hot_water_standard_minimum_thermal_efficiency are functionally identical to the generic Standards versions.

There are other instances where including an extra optional argument would prevent having to overload the whole method with a specific 90.1 PRM version.

In the interest of keeping the codebase smaller and maintainable, this issue is to consolidate and remove such methods.

@lymereJ lymereJ removed their assignment Mar 14, 2023
@lymereJ lymereJ added the AppendixG Methods to enable the Appendix G model workflow label May 5, 2023
@mdahlhausen
Copy link
Collaborator Author

mdahlhausen commented Aug 28, 2023

Copying @lymereJ's comments on duplicated PRM methods:

  • I did not see the “model_custom_hvac_tweaks” methods and I guess more broadly all the methods under prototypes\common\building, we are going to remove them all, right?
  • deep_copy_schedule from ashrae_90_1_prm.SpaceType.rb should probably moved to the utility class that you’re working on
  • I think that the PRM specific “chw_sizing_control” could be revised so we only have one method, it looks like the differences between this one and the one that’s in Standards are default values and the fact that we’re using an OA reset SPM which I see is in a todo note in the Standards version.
  • “planar_surface_apply_standard_construction” could probably be one method with some different references for the PRM, that would save a bit of duplicated code.
  • “model_prm_baseline_system_group_minimum_area” is an interesting one. The PRM specific one has a custom flag for Xcel Energy, but not the standard one. I wonder if these have been inverted? Either way, one of them could be removed since the exception area appears to be the same.
  • Same thing for “model_prm_baseline_system_number” and “model_prm_baseline_system_change_fuel_type”, although the content appears to be different. Maybe they need to be kept, but the Xcel energy custom flag should probably be moved to the standard one.
  • We can probably have one “model_apply_constructions” since the only difference is the default values passed to “model_apply_standard_constructions”
  • We can probably have one “cooling_tower_apply_minimum_power_per_flow”, the only difference between the twos are default values for efficiency lookup
  • “coil_heating_gas_find_capacity” is pretty much identical (the only diff is a comment) so we can definitely have one of them
  • We could potentially have one “chiller_electric_eir_find_search_criteria” method if we make the compressor type lookup a sub method with one for standards and the PRM
  • Only the return is different for “chiller_electric_eir_standard_minimum_full_load_efficiency” so we can have one method for sure
  • “boiler_hot_water_find_search_criteria” is pretty much identical, just default look up values are different but I think they would be the same for the PRM if we only kept the standards method
  • “boiler_hot_water_standard_minimum_thermal_efficiency” is pretty much identical, the only difference are logs
  • “air_terminal_single_duct_vav_reheat_apply_minimum_damper_position” perhaps we could keep the PRM function and toss the standards one? It seems like the only diff is the way we set the MDP, I have a preference for setting the position over setting the flow
  • Maybe 80% of this function “air_loop_hvac_enable_unoccupied_fan_shutoff” is identical between standards and the PRM one, the PRM one has a block at the beginning that look for exception, perhaps this could it its own function and be called at the beginning of “air_loop_hvac_enable_unoccupied_fan_shutoff” and return for a specific value. That way we only have “air_loop_hvac_enable_unoccupied_fan_shutoff”
  • “air_loop_hvac_multizone_vav_optimization_required” are identical
  • “air_loop_hvac_set_vsd_curve_type” are identical
  • “air_loop_hvac_economizer_limits” are pretty much identical, some name reconciliation could happened so we could only keep one of them
  • “model_prm_baseline_system_group_minimum_area” and “model_prm_baseline_system_group_minimum_area”, “model_prm_baseline_system_change_fuel_type”, are defined both in ashrae_90_1_prm.Model and ashrae_90_1_prm_2019, we probably don’t need both
  • “model_baseline_system_vav_fan_type” in ashrae_90_1_prm_2019 is the same as ashrae_90_1_prm, we can probably keep the one in ashrae_90_1_prm

@mdahlhausen
Copy link
Collaborator Author

Thoughts on adding a "set curves" argument for the Standards methods (chiller and boiler curves)?

The ashrae_90_1_prm version of model_apply_standard_constructions is more complete. Consider adopting this in Standards.Model.rb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AppendixG Methods to enable the Appendix G model workflow Developer Issue
Projects
None yet
Development

No branches or pull requests

4 participants