search

NREL / openstudio-model-articulation-gem

Other
7 stars 9 forks source link

changed night cycling control to "Thermostat" cycling and increased t… #60

Closed rawadelkontar closed 3 years ago

rawadelkontar commented 3 years ago

changed night cycling control to "Thermostat" cycling and increased thermostat tolerance to 1.999999

DavidGoldwasser commented 3 years ago

You are also adding this code to the arguments section of the measure here https://github.com/NREL/openstudio-model-articulation-gem/blob/night_cycling_control_to_thermostat/lib/measures/create_typical_building_from_model/measure.rb#L468-L473

If it was added in this measure it would have to go after this line in the run section which does all the work. https://github.com/NREL/openstudio-model-articulation-gem/blob/night_cycling_control_to_thermostat/lib/measures/create_typical_building_from_model/measure.rb#L483

I'm hesitant to make the change in the measure.rb in model articulation because there are three flavors of this measure, and it would be nice for them to be more in sync.

It would be cleaner to make the change in the extension gem in the typical_building_from_model method that all the measures use. https://github.com/NREL/openstudio-extension-gem/blob/develop/lib/openstudio/extension/core/os_lib_model_generation.rb#L2570

@mdahlhausen I'm interested in your option of these three approaches

  1. Only offer the new approach and not the old approach for nigh cycling
  2. Add a new argument that defaults to the new behavior but has option to use old behavior
  3. Add a new argument that defaults to the old behavior but has option to use new behavior

3 is the safest in not impacting research that wants to keep the existing approach, but 1 may be the best to improve the quality of future models.

It may also not be a bad idea to make a simple stand alone measure that does this for general use outside of the automated workflow.

mdahlhausen commented 3 years ago

@DavidGoldwasser I hope to make this change in openstudio-standards, so I think just including it in all models is the way forward.

DavidGoldwasser commented 3 years ago

Thanks @mdahlhausen @kflemin has a pull requests in the extension gem that is passing it's tests, so I'll merge that and I'll close this one without merging it.