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

ScheduleDay::getValue applies interpolation differently than EnergyPlus #5001

Closed
eringold opened this issue Oct 17, 2023 · 7 comments · Fixed by #5111
Closed

ScheduleDay::getValue applies interpolation differently than EnergyPlus #5001

eringold opened this issue Oct 17, 2023 · 7 comments · Fixed by #5111
Assignees

Comments

@eringold
Copy link
Contributor

eringold commented Oct 17, 2023

Issue overview

For a ScheduleDay with 'Interpolate to Timestep' = 'Yes', calling getValue
with a Time value that does not equal one of the scheduled Hour+Minute values will return an interpolated value, which is inconsistent with how EnergyPlus interprets that input.

Current Behavior

EnergyPlus interprets 'Interpolate to Timestep' = 'Average' to mean "when the simulation timestep doesn't coincide with the scheduled interval, the value at the intermediate timestep is interpolated". I.e., for this schedule:

Schedule:Day:Interval,
    Day Schedule,            !- Name
    Fractional,              !- Schedule Type Limits Name
    Average,                 !- Interpolate to Timestep
    8:15,            	     !- Time 1 {hh:mm}
    0,                       !- Value Until Time 1
    21:45,                   !- Time 2 {hh:mm}
    1,                       !- Value Until Time 2
    24:00,                   !- Time 3 {hh:mm}
    0;                       !- Value Until Time 3

If the simulation timesteps per hour are 4, will evaluate like:
image

With 'Average' interpolation, for 10 timesteps per hour, it would evaluate like:
image

However ScheduleDay::getValue in OpenStudio interpolates between schedule times (i.e., 8:15 to 21:45), so calling getValue with intermediate times results in a plot like this:
image

Expected Behavior

I expect getValue to return the same value that EnergyPlus would for a schedule at a given time.

Steps to Reproduce

Create a ScheduleDay with 'Interpolate To Timestep' = 'Yes', and make successive calls to getValue like:

def print_vals(sch)
  (0..24).each do |hr|
    ["00","15","30","45"].each do |min|
      hr_pad = "%02d" % hr
      time = OpenStudio::Time.new("#{hr_pad}:#{min}:00")
      val = day_sch.getValue(time)
      puts "#{time}, #{val}"
    end
  end
end

model = OpenStudio::Model::Model.new
sch = OpenStudio::Model::ScheduleDay.new(model)
sch.setInterpolatetoTimestep(true)
sch.addValue(OpenStudio::Time.new("08:15:00"),0)
sch.addValue(OpenStudio::Time.new("21:45:00"),1)
print_vals(sch)

-> ...
08:00:00, 0.0
08:15:00, 0.0
08:30:00, 0.01851851851851845
08:45:00, 0.03703703703703701
09:00:00, 0.05555555555555555
...
21:00:00, 0.9444444444444444
21:15:00, 0.9629629629629629
21:30:00, 0.9814814814814814
21:45:00, 1.0
22:00:00, 0.8888888888888893
22:15:00, 0.7777777777777786

Some additional details about your environment for this issue (if relevant):

  • Platform (Operating system, version): all
  • Version of OpenStudio (if using an intermediate build, include SHA): 3.6.1 but this goes back a long ways

Context

This impacts multiple uses of getValue, e.g. for determining HVAC occupancy schedules in openstudio-standards, schedule reporting in OpenStudio Results, etc.

@eringold eringold added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Oct 17, 2023
@DavidGoldwasser DavidGoldwasser removed the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Oct 17, 2023
@joseph-robertson
Copy link
Collaborator

joseph-robertson commented Oct 18, 2023

For users, is the alternative/workaround here to add every single value to the day schedule for all timesteps?

@eringold
Copy link
Contributor Author

@joseph-robertson I suppose. Alternatively, the user could store the value of interpolatetoTimestep, set it to false before calling getValue, and reset it after. That still wouldn't exactly match EnergyPlus, but would be closer than the existing implementation.

@eringold
Copy link
Contributor Author

eringold commented Mar 5, 2024

@joseph-robertson

For users, is the alternative/workaround here to add every single value to the day schedule for all timesteps?

How about a new OS:ScheduleDay method getValuesatTimestep(OS:Timestep) that returns a vector of schedule values, which you could then interpolate for a given OS:Time? It would be useful to replace this standards method, and (ideally) extend it up to ScheduleRuleset, like this

@DavidGoldwasser @mdahlhausen

@joseph-robertson
Copy link
Collaborator

joseph-robertson commented Mar 13, 2024

Hi @eringold, can you have a look at #5111 and tell me if this is what you had in mind?

This is a method to return all values of a day schedule, regardless of the interpolatetoTimestep setting. Users would then have the ability, for a given timestep interval, to apply whatever interpolation approach they want?

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2024

Just noticed this

bool interpolatetoTimestep() const;

Maybe it'd be good to change it to be a choice field like E+, and bring the os-standards average method into C++

  A3 , \field Interpolate to Timestep
       \note when the interval does not match the user specified timestep a Average choice will average between the intervals request (to
       \note timestep resolution. A No choice will use the interval value at the simulation timestep without regard to if it matches
       \note the boundary or not. A Linear choice will interpolate linearly between successive values.
       \type choice
       \key Average
       \key Linear
       \key No

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2024

/** Enum to specify the interpolation method. */
enum InterpMethod
{
LinearInterp,
NearestInterp,
HoldLastInterp,
HoldNextInterp
};

@jmarrec
Copy link
Collaborator

jmarrec commented Mar 14, 2024

double ScheduleDay_Impl::getValue(const openstudio::Time& time) const {
if (time.totalMinutes() < 0.0 || time.totalDays() > 1.0) {
return 0.0;
}
std::vector<double> values = this->values(); // these are already sorted
std::vector<openstudio::Time> times = this->times(); // these are already sorted
unsigned N = times.size();
OS_ASSERT(values.size() == N);
if (N == 0) {
return 0.0;
}
openstudio::Vector x(N + 2);
openstudio::Vector y(N + 2);
x[0] = -0.000001;
y[0] = 0.0;
for (unsigned i = 0; i < N; ++i) {
x[i + 1] = times[i].totalDays();
y[i + 1] = values[i];
}
x[N + 1] = 1.000001;
y[N + 1] = 0.0;
InterpMethod interpMethod;
if (this->interpolatetoTimestep()) {
interpMethod = LinearInterp;
} else {
interpMethod = HoldNextInterp;
}
double result = interp(x, y, time.totalDays(), interpMethod, NoneExtrap);
return result;
}

/// linear interpolation of the function y = f(x) at point xi
/// assumes that x is strictly increasing
double interp(const Vector& x, const Vector& y, double xi, InterpMethod interpMethod, ExtrapMethod extrapMethod) {
size_t N = x.size();
double result = 0.0;
if (y.size() != N) {
return result;
}
InterpInfo info = interpInfo(x, xi);
if (info.extrapolated) {
switch (extrapMethod) {
case NoneExtrap:
// set to zero
result = 0.0;
break;
case NearestExtrap:
// pick closest point
result = (info.wa > info.wb ? y(info.ia) : y(info.ib));
break;
}
} else {
switch (interpMethod) {
case LinearInterp:
// linear interpolation
result = info.wa * y(info.ia) + info.wb * y(info.ib);
break;
case NearestInterp:
// pick closest point
result = (info.wa > info.wb ? y(info.ia) : y(info.ib));
break;
case HoldLastInterp:
// set to previous value
result = y(info.ia);
break;
case HoldNextInterp:
// set to next value
result = y(info.ib);
break;
}
}
return result;
}

doesn't the getValues routine look inefficient? If I'm understanding correctly, you only need to find the time before and the time after and the corresponding, you can break early. I know, this is a scheduleday, so we're talking about 60 (timestep max in E+ If I recall correctly) * 24 hours datapoints max, so it's not noticeable, but still

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 a pull request may close this issue.

4 participants