-
Notifications
You must be signed in to change notification settings - Fork 396
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
|
|
|
Hmm, there is no Zone Minimum CO2 Concentration schedule in DOAToPTHP or DOAToUnitarySystem so these files don't run in this branch. There is also no Controller:MechanicalVentilation object in these files.
|
Correct. Why is okay for this object to refer to schedules that don't exist but not okay for any other object? |
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.
|
Pull request overview
Details forthcoming. Just testing for now.
Fixes #10869