NREL / openstudio-common-measures-gem

Other
8 stars 15 forks source link

Changes will be required to OpenStudio Results for measure for OpenStudio 3.1/EnergyPlus 9.4 #33

Closed DavidGoldwasser closed 4 years ago

DavidGoldwasser commented 4 years ago

@jmarrec hi-lights some of this in openstudio3 slack channel.

The changes should be made here https://github.com/NREL/openstudio-common-measures-gem/blob/develop/lib/measures/openstudio_results/resources/os_lib_reporting.rb

There is a copy of this in the extension gem that may create conflicts, unless I keep that in sync with this I should remove it from extension gem. OpenStudio results has been working on older versions of OpenStudio that do not yet have extension gem installed. https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/core/os_lib_reporting.rb

DavidGoldwasser commented 4 years ago

As a note "os_lib_reporting" is not a general use library file like the others, is used by one or 2 other measures in a private repo that are slightly modified and probably outdated version of OpenStudio results. My preference at the moment is to remove it from extension gem, and if other measures are published, they should use unique module name and embed the file with the measure.

@nllong any thoughts on this, sorry for cross repo conversation.

macumber commented 4 years ago

I agree with this @DavidGoldwasser. The OpenStudio Results measure includes a bunch of sizing models from from OpenStudio Standards but those have not been kept up to date. I started a pattern in the extension gem where you would try to load files from any active gems first and then fall back to files in the resources directory as a backup:

https://github.com/NREL/openstudio-extension-gem/blob/e354355054b83ffc26e3b69befa20d6baf5ef242/lib/measures/openstudio_extension_test_measure/measure.rb#L36

The files in the backup should be brought up to current OpenStudio Standards. Are you also going to update to reflect the changes in E+ 9.4 output variables? I think FuelOil is going through yet another renaming.....

DavidGoldwasser commented 4 years ago

@macumber yes I'm going to update for changes in 9.4, Thanks for heads up about another FuelOil change. I'm going to use this branch that I already made for some enhancements to the measure. https://github.com/NREL/openstudio-common-measures-gem/tree/os_results_warn_enhance

@mdahlhausen and @asparke2 are all of those Size.Object still necessary here or can they be called directly from standards. Note those files are not in openstudio-extension gem now, only in this measure. https://github.com/NREL/openstudio-common-measures-gem/tree/os_results_warn_enhance/lib/measures/openstudio_results/resources

DavidGoldwasser commented 4 years ago

I'll likely increase <min_compatible>3.0.0</min_compatible> to 3.1.0 vs. trying to support EnergyPlus 9.3 and 9.4

macumber commented 4 years ago

Looks like there is a tag and release for this repo to go with OS 3.0: https://github.com/NREL/openstudio-common-measures-gem/releases/tag/v0.2.0

Can you make another tag and release to go with OS 3.1?

asparke2 commented 4 years ago

@DavidGoldwasser yes, the Siz.Object files are still needed because the methods they implement are not in the C++.

DavidGoldwasser commented 4 years ago

@macumber Yes, I will make another tag and release for OS 3.1 but it will probably fall a few days to a week after the official release of OpenStudio 3.1

DavidGoldwasser commented 4 years ago

Other measures that have test failure with 3.1 are

RadiantSlabWithDoas (this may be fixed with standards gem update)

I've made progress on OpenStudio Results but still working on that and updates to OS identified by measure failures.

jmarrec commented 4 years ago

@macumber Yes, I will make another tag and release for OS 3.1 but it will probably fall a few days to a week after the official release of OpenStudio 3.1

Isn't this gem embedded in the cli? It might not be, sometimes i loose track

DavidGoldwasser commented 4 years ago