Repository navigation
Add new Schedule:Year:Rules detailed schedule type - #11693
Conversation
… vector, default-initialize in WeekRuleSchedule struct.
…lean error handling, etc.
…already included in test setup.
|
@joseph-robertson I think this one is ready. @kbenne @jmarrec anything from you, or anyone else? |
|
|
||
| ErrorObjectHeader eoh{routineName, CurrentModuleObject, Alphas(1)}; | ||
|
|
||
| if (s_sched->weekRuleScheduleMap.find(Util::makeUPPER(Alphas(1))) != s_sched->weekRuleScheduleMap.end()) { |
There was a problem hiding this comment.
I see a buckload of calls to Util::makeUPPER in this file. They are all probably redundant/moot.
At least these with Alphas() are.
The InputProcessor already uppercases every alpha field when populating Alphas, unless the corresponding IDD field is marked \retaincase.
EnergyPlus/src/EnergyPlus/InputProcessing/InputProcessor.cc
Lines 735 to 749 in 18f677f
None of the relevant fields in idd/Energy+.idd.in (Name fields for Schedule:Week:Daily, Schedule:Week:Rule, Schedule:Year, Schedule:Year:Rules, Schedule:Compact, Schedule:File, Schedule:Constant, the ExternalInterface:*Schedule objects, etc.) carry \retaincase, so Alphas(N) arrives already uppercase and the extra makeUPPER() is useless
There was a problem hiding this comment.
I think this was motivated by #11693 (comment). Does this mean we could also remove all the Util::makeUPPER in, e.g., s_sched->weekRuleScheduleMap.insert_or_assign(Util::makeUPPER(weekRuleSched->Name), weekRuleSched->Num);?
There was a problem hiding this comment.
I left the insert_or_assign(Util::makeUPPER(...), ...);.
There was a problem hiding this comment.
(Yes, it could have gone away too)
| if ((Alphas(4)) == "YES") { | ||
| weekRuleSched->applySunday = true; | ||
| } |
There was a problem hiding this comment.
Another proof that it's already uppercase...
|
|
||
| CurrentModuleObject = "Schedule:Week:Rule"; | ||
| for (int Loop = 1; Loop <= NumWeekRuleSchedules; ++Loop) { | ||
| s_ip->getObjectItem(state, |
There was a problem hiding this comment.
@mitchute As a side note, at some point there should be clarity / developer guidelines on when we should use getObjectItem (with ObjexxFCL arrays) versus using the epJSON directly.
Should new GetInput code use epJSON? Even if the rest of the file it's added in uses the old (legacy?) getObjectItem.
There was a problem hiding this comment.
Agreed. I think #11756 is related to this, but this is something we should get aligned on very soon.
|
|
| \paragraph{Field: Name}\label{field-name-5-021} | ||
|
|
||
| This field should contain a unique (between Schedule:Year, \hyperref[schedulecompact]{Schedule:Compact}, and \hyperref[schedulefile]{Schedule:File}) designation for the schedule. It is referenced by various ``scheduled'' items (e.g.~\hyperref[lights-000]{Lights}, \hyperref[people]{People}, Infiltration) to define the appropriate schedule values. | ||
| This field should contain a unique (between Schedule:Year, \hyperref[schedulecompact]{Schedule:Compact}, \hyperref[scheduleyearrules]{Schedule:Year:Rules}, and \hyperref[schedulefile]{Schedule:File}) designation for the schedule. It is referenced by various ``scheduled'' items (e.g.~\hyperref[lights-000]{Lights}, \hyperref[people]{People}, Infiltration) to define the appropriate schedule values. |
There was a problem hiding this comment.
The wording is a bit awkward on these, but that's not your problem. Maybe in the future we can implement some semi-automated, systematic approach to working though the docs for language and style cleanups.
|
Last call here, too. Thanks for all the work on this @joseph-robertson. |
|
Alrighty. Ready on this one. Merging. |
Pull request overview
Schedule:Year:RulesandSchedule:Week:Ruleobjects for describing detailed schedules.Description of the purpose of this PR
Pull Request Author
Reviewer