-
Notifications
You must be signed in to change notification settings - Fork 397
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
Schedule API #10848
base: develop
Are you sure you want to change the base?
Schedule API #10848
Conversation
|
auto &s_sched = state.dataSched; | ||
for (int i = 0; i < (int)s_sched->scheduleTypes.size(); ++i) if (s_sched->scheduleTypes[i]->Name == name) return i; | ||
return -1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[src/EnergyPlus/ScheduleManager.cc:99]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:227]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:241]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:273]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:385]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2409]:(style),[constParameterReference],Parameter 'state' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2497]:(style),[constParameterReference],Parameter 'state' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2502]:(style),[constParameterReference],Parameter 'state' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2515]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2553]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2587]:(style),[constVariableReference],Variable 's_sched' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2725]:(style),[constParameterReference],Parameter 'state' can be declared as reference to const
[src/EnergyPlus/ScheduleManager.cc:2978]:(style),[constVariableReference],Variable 's_glob' can be declared as reference to const
I noticed that Schedule Value was added to the rdd for 1ZoneUncontrolled_win_1.idf. There are no schedules in that file. There is a ScheduleTypeLimits object.
The error file shows there are 2 unused schedules:
|
Hmmmm. I think I know what this is, but I will verify after I am done looking at something more serious. In this refactor, |
|
|
|
|
|
|
…e vectors and not just arrays
|
|
|
This does not reproduce locally on macos. The actual value is 2479.5 on both develop and scheduleAPI branch and rounds up to 2480. on both. Maybe on Windows or another system one of the values actually computes as 2479.4999999999 and rounds down? develop does a sum on all schedule values during this calculation while scheduleAPI does a sum on day schedule values first and then sums those, so a change in order of floating-point operations often results in issues like these. Not sure there is a real issue here other than just spurious diffs that need to be sorted through. 🤷 |
|
|
|
1ZoneDataCenterCRAC_* all show the same diff with ITE equipment. I would think the Max would show a non-zero number but it depends on the schedules for those day types. These 8 day types show diffs in the min/max. For all these to show 0 -> 50000 seems like a need to investigate.
And then I saw this and think this branch fixed something:
These 8 days types show diffs: Minimum Equipment Level for Weekdays {W}, Maximum Equipment Level for Weekdays {W},Minimum Equipment Level for Weekends/Holidays {W}, Maximum Equipment Level for Weekends /Holidays {W},Minimum Equipment Level for Summer Design Days {W}, Maximum Equipment Level for Summer Design Days {W},Minimum Equipment Level for Winter Design Days {W}, Maximum Equipment Level for Winter Design Days {W}
|
This is a bug in develop. The day schedule MaxTSVal and MinTSVal were not set before this report was printed so they show up as zeros. |
|
@@ -892,10 +887,9 @@ void GetPumpInput(EnergyPlusData &state) | |||
|
|||
// Input the optional schedule for the pump | |||
if (thisInput->cAlphaArgs(6).empty()) { // Initialized to zero, don't get a schedule for an empty | |||
thisPump.flowRateSched = Sched::GetScheduleAlwaysOn(state); | |||
thisPump.flowRateSched = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already null
Sched::Schedule *flowRateSched = nullptr; // Flow rate modifier schedule, blank/missing --> AlwaysOn
|
RotSpeed_Min = thisPump.VFD.minRPMSched->getCurrentVal(); | ||
RotSpeed_Max = thisPump.VFD.maxRPMSched->getCurrentVal(); | ||
RotSpeed_Min = thisPump.VFD.minRPMSched ? thisPump.VFD.minRPMSched->getCurrentVal() : 0.0; | ||
RotSpeed_Max = thisPump.VFD.maxRPMSched ? thisPump.VFD.maxRPMSched->getCurrentVal() : 0.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised this would crash. If HasVFD = true then min/maxRPMSched must be valid schedules.
|
|
src/EnergyPlus/RefrigeratedCase.cc
Outdated
@@ -2242,7 +2242,7 @@ void GetRefrigerationInput(EnergyPlusData &state) | |||
} | |||
|
|||
++AlphaNum; // A6 | |||
if (lAlphaBlanks(AlphaNum)) { | |||
if (!lAlphaBlanks(AlphaNum)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to bisect something I was seeing locally so I reverted this line. I didn't work.
|
|
@@ -852,7 +852,7 @@ | |||
Until: 24:00,23.89, !- Field 23 | |||
For: Sunday Holidays AllOtherDays, !- Field 25 | |||
Until: 8:00,29.44, !- Field 26 | |||
Until: 8:00,26.67, !- Field 28 | |||
Until: 8:00,26.67, !- Field 28 Is this supposed to be 9:00? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the occupancy schedule pattern for Type1, Type2 and Type3 where Type1 occupancy occurs at 8 AM this look correct.
Schedule:Compact,
Type1_OCC_SCH, !- Name
For: Sunday Holidays AllOtherDays, !- Field 50
Until: 08:00,0.0, !- Field 51
Until: 10:00,0.05, !- Field 53
Schedule:Compact,
Type2_OCC_SCH, !- Name
For: Sunday Holidays AllOtherDays, !- Field 52
Until: 09:00,0.0, !- Field 53
Until: 10:00,0.05, !- Field 55
Schedule:Compact,
Type3_OCC_SCH, !- Name
For: Sunday Holidays AllOtherDays, !- Field 62
Until: 10:00,0.0, !- Field 63
Until: 11:00,0.11, !- Field 65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does? Two consecutive "Until 8:00, X" statements is correct? At the very least one of them is redundant. Which one though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two consecutive "Until 8:00, X" statements is NOT correct input syntax (but it doesn't hurt anything). The point is that the 2nd 8:00 should not be 9:00 since occupancy starts at 8:00. I think the developer noticed that occupancy started at 8:00 and simply changed the time on the 2nd line from 9 to 8. So the question is did the developer want 29.44 C or 26.67 C as the Tstat temp from midnight to 8:00? Either way the occupied Tstat setting of 23.89 C should begin at 8:00.
For: Sunday Holidays AllOtherDays, !- Field 25
Until: 8:00,29.44, !- Field 26
Until: 8:00,26.67, !- Field 28
Until: 23:00,23.89, !- Field 30
Until: 24:00,23.89; !- Field 32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe the first Until 8:00
should really be Until 7:00
?
Pull request overview
Details forthcoming. Just testing for now.
Fixes #10869