Daylight savings time adding incorrect month calculation in time of use tariff

[JasonGlazer]

Issue overview

The UtilityCost:Tariff object with a time of use period schedule and a season schedule that both start the summer on June 1 are showing summer tariff calculations starting in May. This file has a RunPeriodControl:DaylightSavingTime which starts daylight savings saving in March. When the RunPeriodControl:DaylightSavingTime is removed, the summer calculations are started in June when it is supposed to be.

Details

Some additional details for this issue (if relevant):

• EnergyPlus 9.2.0
• https://unmethours.com/question/42857/utility-tariff-not-calculating-midpeak-the-first-month/

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

• Defect file added (devsupport)
• Ticket added to Pivotal for defect (development team task)
• Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)

[jmarrec]

After fighting with my debugger, I think I've pinpointed something: it seems that ScheduleManager::GetCurrentScheduleValue returns something different than ScheduleManager::LookUpScheduleValues

To better illustrate my point, I've taken this block:

EnergyPlus/src/EnergyPlus/EconomicTariff.cc

Lines 2775 to 2777 in 36db22b

	if (tariff(iTariff).seasonSchIndex != 0) {
	curSeason = GetCurrentScheduleValue(tariff(iTariff).seasonSchIndex);
	} else {

And replaced it with

if (tariff(iTariff).seasonSchIndex != 0) {
    curSeason = GetCurrentScheduleValue(tariff(iTariff).seasonSchIndex);
    if (curSeason == 3) {
       Real64 valueViaLookup = ScheduleManager::LookUpScheduleValue(
               tariff(iTariff).seasonSchIndex, DataGlobals::HourOfDay , DataGlobals::TimeStep);
       if (int(valueViaLookup) != curSeason) {
           ShowFatalError("GatherForEconomics: Something went terribly wrong");
       }
    }
}

Running with the defect file (or the MCVE I created) that has DST, and a season schedule like this:

Schedule:Compact,
    Electricity Season Schedule,  !- Name
    Any Number,              !- Schedule Type Limits Name
    Through: 5/31,           !- Field 1
    For: AllDays,            !- Field 2
    Until: 24:00,            !- Field 3
    1,                       !- Field 4
    Through: 9/30,           !- Field 5
    For: AllDays,            !- Field 6
    Until: 24:00,            !- Field 7
    3,                       !- Field 8
    Through: 12/31,          !- Field 9
    For: AllDays,            !- Field 10
    Until: 24:00,            !- Field 11
    1;                       !- Field 12

The Fatal is issued. GetCurrentScheduleValue returns 3 (summer), while LookUpScheduleValue returns correctly 1 (winter).

[rraustad]

This does ring a bell. Just gotta remember where.

[rraustad]

I was thinking of #7462. Doesn't seem the same as your simple schedule.

[mjwitte]

@rraustad It looks like the changes applied in #7462 were only made in UpdateScheduleValues (which feeds into GetCurrentScheduleValue) but not in LookupScheduleValue?

@jmarrec It would be helpful to know the day/hour/timestep when the fatal test trips.

[jmarrec]

@rraustad @mjwitte I isolated it in a unit test, cf 347707d

[mjwitte]

So, the question is which one is correct? #7462 would say that GetCurrentScheduleValue is the right answer. Clearly, both functions should return the same schedule value, but perhaps the economics calcs need to careful about when to select a monthly value? @JasonGlazer

[jmarrec]

ScheduleManager.cc has three routines with similar blocks of code.
These two are exactly the same (minus whitespace changes)

• ScheduleManager::ReportScheduleValues (called from HeatBalanceManager::ReportHeatBalance)
• ScheduleManager::UpdateScheduleValues (called from WeatherManager::SetCurrentWeather)

This one is much different: ScheduleManager::LookUpScheduleValues. It seems be doing stuff twice on some occasions (control flow can probably be improved), and appears to add the DSTIndicator twice. I'm having a hard time following it right now, but will hopefully get it tomorrow morning after some rest.

EnergyPlus/src/EnergyPlus/ScheduleManager.cc

Line 2922 in 36db22b

WhichHour = HourOfDay + DSTIndicator;

EnergyPlus/src/EnergyPlus/ScheduleManager.cc

Line 2954 in 36db22b

WhichHour += DSTIndicator;

[jmarrec]

This block appears to produce no meaningful actions as it's all overwritten later no matter the branches taken.

EnergyPlus/src/EnergyPlus/ScheduleManager.cc

Lines 2910 to 2927 in 36db22b

	// Determine which Week Schedule is used
	// Cant use stored day of year because of leap year inconsistency
	WeekSchedulePointer = Schedule(ScheduleIndex).WeekSchedulePointer(DayOfYear_Schedule);
	// Now, which day?
	if (DayOfWeek <= 7 && HolidayIndex > 0) {
	DaySchedulePointer = WeekSchedule(WeekSchedulePointer).DaySchedulePointer(7 + HolidayIndex);
	} else {
	DaySchedulePointer = WeekSchedule(WeekSchedulePointer).DaySchedulePointer(DayOfWeek);
	}
	// Hourly Value
	WhichHour = HourOfDay + DSTIndicator;
	if (WhichHour <= 24) {
	LookUpScheduleValue = DaySchedule(DaySchedulePointer).TSValue(TimeStep, WhichHour);
	} else {
	LookUpScheduleValue = DaySchedule(DaySchedulePointer).TSValue(TimeStep, WhichHour - 24);
	}

[mjwitte]

@jmarrec FYI, I wouldn't get too carried away with any refactoring here beyond fixing the issue. There's a major schedule manager refactor in progress #7352. Wonder how that branch handles this case?

[JasonGlazer]

@mjwitte the tariff stuff ought to add up the values for a time period with daylight savings time account for. Nothing special about that.

@jmarrec there does seem to be some redundancy in the code but be careful, there are a lot of edge cases.

[mjwitte]

@JasonGlazer What isn't clear here, is when in May the summer calcs are starting - is it just for that last hour on May 31?

[JasonGlazer]

I don't remember for sure but I assume it was just an hour off in May.

[mjwitte]

Then perhaps it's correct as-is. If the utility is collecting meter data hourly (or more often), then the last day of standard time will only have 23 hours of data, and the last day of DST would have 25. If I'm following this correctly, that seems to be what's happening here. There's still the issue of the two different functions returning different results, but that seems to be unrelated to tariff calcs.

[jmarrec]

The entire month of may ends up billed at the summer rate, so the error isn't just one hour off.

IIUC Tariff(n).seasonForMonth(CurMonth) is constantly overriden and only the last one to get in is used to compute the cost.

I'll try the ScheduleManager refactor branch tomorrow.

I do think GetScheduleValue and LookUpScheduleValue should return the same though. The different is the determination of the week/day schedule pointers, in one case(LookUp) it's done with HourOfDay then after determination is done DSTIndicator is added, for the other it's directly done with HourOfDay + DSTIndicator