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

Systematic warnings with setShadingControlType + setSetpoint(s) #4911

Closed
brgix opened this issue Jun 6, 2023 · 2 comments · Fixed by #4914
Closed

Systematic warnings with setShadingControlType + setSetpoint(s) #4911

brgix opened this issue Jun 6, 2023 · 2 comments · Fixed by #4914

Comments

@brgix
Copy link

brgix commented Jun 6, 2023

Enhancement Request (low priority)

User changes to ShadingControl control type (e.g. from the default "OnIfHighSolarOnWindow") and/or to required setpoint(s) trigger one or more warnings to STDOUT (for instance a systematic resetSetpoint, triggering this warning:

LOG(Warn, briefDescription() << " has a Shading Control Type '" << shadingControlType << "' which does require a Setpoint, not resetting it");

IMO, these warnings are atypical for the SDK, unnecessary, and a source of confusion.

IRB steps to trigger STDOUT warnings:

require 'openstudio'
model = OpenStudio::Model::Model.new
shd = OpenStudio::Model::Shade.new(model)
ctl = OpenStudio::Model::ShadingControl.new(shd)
ctl.setSetpoint(18)
ctl.setShadingControlType("OnIfHighOutdoorAirTempAndHighSolarOnWindow")

or

require 'openstudio'
model = OpenStudio::Model::Model.new
shd = OpenStudio::Model::Shade.new(model)
ctl = OpenStudio::Model::ShadingControl.new(shd)
ctl.setSetpoint(18)
ctl.setSetpoint2(100)
ctl.setShadingControlType("OnIfHighOutdoorAirTempAndHighSolarOnWindow")

Possible Implementation

The simplest would be to cease generating such warnings altogether. There are numerous OpenStudio objects that are generated without a complete set of attributes (or maybe conflicting ones), only to be discovered when attempting to run an E+ simulation. Users aren't warned in advance; more of a consumer beware approach. It's great to rely on returned booleans to signal successful/failed attempts to re/set attributes, but I would prefer less verbose assistance such as the warnings discussed here.

A more nuanced solution would be to provide method overrides with additional parameters as needed (without triggering warnings), e.g.:

ctl.setShadingControlType("OnIfHighOutdoorAirTempAndHighSolarOnWindow", 18, 100)

... possibly even with default parameter values if missing.

Possible BUG

Not 100% certain, but I think this is a bug:

if (ShadingControl_Impl::isControlTypeValueNeedingSetpoint1(shadingControlType)) {

... should it not be:

if (ShadingControl_Impl::isControlTypeValueNeedingSetpoint2(shadingControlType))

@brgix brgix added Enhancement Request Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Jun 6, 2023
@jmarrec
Copy link
Collaborator

jmarrec commented Jun 12, 2023

You're right about the bug part.

As far as the warnings being annoying... Not sure I'm totally with you on this.

  1. Warnings are there to help you understand what's going on. Here we're trying to enforce object correctness at model time, so we do reset some stuff and since this may confusing, we also log a message.

  2. "There are numerous OpenStudio objects that are generated without a complete set of attributes (or maybe conflicting ones), only to be discovered when attempting to run an E+ simulation". Well in some cases that is true, but that's not generally the intent of the SDK. Sometimes we just can't, or just won't (some less used/advanced objects don't necesarilly deserve too much engineering).

  3. If you're annoyed, just turn off the logging (or set the log level to Error) around the block that does it. Or set a negative lookbehind filter to exclude ShadingControl globally. This should do it: OpenStudio::Logger.instance.standardOutLogger.setChannelRegex('.*(?<!ShadingControl)$')

[1] model(main)> OpenStudio::logFree(OpenStudio::Warn, "openstudio.model.Other", "Warn")
[openstudio.model.Other] <0> Warn
=> nil
[2] model(main)> OpenStudio::logFree(OpenStudio::Warn, "openstudio.model.ShadingControl", "Warn")
[openstudio.model.ShadingControl] <0> Warn
=> nil
[3] model(main)> OpenStudio::Logger.instance.standardOutLogger.setChannelRegex('.*(?<!ShadingControl)$')
=> nil
[4] model(main)> OpenStudio::logFree(OpenStudio::Warn, "openstudio.model.Other", "Warn")
[openstudio.model.Other] <0> Warn
=> nil
[5] model(main)> OpenStudio::logFree(OpenStudio::Warn, "openstudio.model.ShadingControl", "Warn")
=> nil
  1. This block can be eliminated though
    // TODO: do we want to force remove this stuff like it did resetSetpoint before? Units/quantity might be oranges and apples when switching
    // types, but they could be compatible, and it may be very confusing at least for interface users that are playing with a dropdown to see
    // schedulesand setpoints disapear
    if (!openstudio::istringEqual(oldControlType, shadingControlType)) {
    resetSetpoint(); // For backward compatibility
    // resetSetpoint2();
    // resetSchedule();
    }
    . And you'd be left with Info messages, not a Warn, so they'd typically be filtered out (by default they will).

jmarrec added a commit that referenced this issue Jun 12, 2023
@jmarrec jmarrec self-assigned this Jun 12, 2023
@jmarrec jmarrec added component - Model and removed Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Jun 12, 2023
@brgix
Copy link
Author

brgix commented Jun 12, 2023

@jmarrec : thanks for the logger tips!

jmarrec added a commit that referenced this issue Jun 29, 2023
Fix #4911 - Avoid extra warning message in ShadingControl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants