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

Remove obsolete OpenStudio Model Object autosizing and hardsizing methods #1410

Merged
merged 8 commits into from
Feb 13, 2023

Conversation

asparke2
Copy link
Member

Methods to get autosized values for component capacities, flow rates, etc. were needed by OpenStudio-Standards in order to apply code-baseline efficiencies and controls. These methods were originally implemented in Ruby inside OpenStudio-Standards for expedience and prototyping. In OpenStudio 2.5.0 (released 3/30/2018), these methods were added to the OpenStudio C++ API, rendering the Ruby implementation obsolete. However, the OpenStudio-Standards implementation was kept for backwards compatibility. Today, the expectation is that most users of OpenStudio-Standards are using a version of OpenStudio >= 2.5.0, making the Ruby implementation unnecessary.

This pull request removes the redundant Ruby methods that are present (with the same method names and signatures) in the OpenStudio C++ API. After this change, Measures or other code using require 'openstudio-standards' will be using the C++ methods maintained by the OpenStudio development team. This will both reduce the maintenance burden on OpenStudio-Standards and reduce confusion about the source of the problem if a method is not working properly.


end
# returns the autosized design coil load
def autosizedDesignCoilLoad
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this method remain?

Copy link
Collaborator

@mdahlhausen mdahlhausen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should any of the methods in these files be moved to OpenStudio?:

  • Siz.AirLoopHBAC.rb
  • Siz.CoilCoolingWater.rb
  • Siz.HeatingCoolingFuels.rb
  • Siz.HVACComponent.rb
  • Siz.Model.rb
  • Siz.ThermalZone.rb

If not, can they be moved to elsewhere in standards? (e.g. heating and cooling fuels make sense in utilities)

Any reason why the necb/NRCAN methods weren't updated to direct methods in OpenStudio?

@asparke2
Copy link
Member Author

asparke2 commented Feb 3, 2023

Not using the component-specific methods that exist was an oversight, those have been corrected.

I made a feature request in OpenStudio to implement the other outstanding methods in C++, but in the meantime will keep the implementation as extensions to OpenStudio::Model objects inside /hvac_sizing so that when the C++ implementation happens, the methods are easy to find and swap out.

  • Siz.AirLoopHBAC.rb, Siz.CoilCoolingWater.rb, Siz.ThermalZone.rb

    • The methods in these files probably should be implemented in C++. They weren't, either because they are methods that don't correspond to input fields found in the objects, or because they come from a sizing-related object such as Sizing:Zone that controls another object such as ThermalZone.
  • Siz.HeatingCoolingFuels.rb, Siz.HVACComponent.rb

    • These methods could be implemented in C++, but the developers will need to keep a close eye when new objects are added because these methods can be difficult to test.
  • Siz.Model.rb

    • Simply loads the other files. Will be unnecessary when methods are in C++.

@mdahlhausen
Copy link
Collaborator

@asparke2 looks like the latest commit introduced an error in the NECB workflow

@mdahlhausen mdahlhausen merged commit 95e5575 into master Feb 13, 2023
@mdahlhausen mdahlhausen deleted the remove_autosizing_methods branch February 13, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants