-
Notifications
You must be signed in to change notification settings - Fork 192
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
I believe that "Linear" is the correct replacement for "Yes"
- Loading branch information
Showing
2 changed files
with
6 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2c44625
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.
Maybe. But then is the transition program incorrect or did I misinterpret?
https://github.com/NREL/EnergyPlus/blob/develop/src/Transition/CreateNewIDFUsingRulesV8_9_0.f90#L511
2c44625
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.
On another note. Typically when there is a field name change I try to comment in the oxygen comments of the OS API saying something like "As of E+ version x.y.z, this field maps to the field named blah blah"
I did not make a comment here, but perhaps we should say something about the mapping.
Maybe also add the new keys to the OS idd?
2c44625
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 discussed this with @jasondegraw yesterday, maybe we need to pull @JasonGlazer in to discuss? I am unclear what the motivation for this change is, do you know where the detailed documentation of this change is?
2c44625
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.
https://github.com/NREL/EnergyPlus/blob/develop/doc/input-output-reference/src/overview/group-schedules.tex#L129
2c44625
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.
Three possible inputs are available for this field: Average, Linear, and No. The default value is No.
If
Average'' is entered, it is used to apply values that aren't coincident with the given timestep (ref: Timestep) intervals. If
Average'' is entered, then any intervals entered here will be interpolated/averaged and that value will be used at the appropriate minute in the hour. For example, if ``Average'' is entered and the interval is every 15 minutes (say a value of 0 for the first 15 minutes, then .5 for the second 15 minutes) AND there is a 10 minute timestep for the simulation: the value at 10 minutes will be 0 and the value at 20 minutes will be .25. In earlier versions of EnergyPlus, this was the Yes option for the field.If
No'' is entered, then the value that occurs on the appropriate minute in the hour will be used as the schedule value. For the same input entries but
no'' for this field, the value at 10 minutes will be 0 and the value at 20 minutes will be .5. No is the default for this field.If ``Linear'' is entered, then the value that is used is based on the linear interpolation between successive values. With linear, if the value at 1:00 is 0.0 and the value at 2:00 is 10.0, with fifteen minute timesteps, the value at 1:15 would be 2.5, the value at 1:30 would be 5.0 and the value at 1:45 would be 7.5.
2c44625
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.
Ok went back to Average for forward translation here: 29a61f6
On import, mapping both linear and average to yes since we do not support the new keys
2c44625
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 used to be "Yes" should now be "Average", "No" remains the same, and "Linear" is something new that probably would have been what people expected but is actually brand new. See:
https://github.com/NREL/EnergyPlus/blob/develop/src/Transition/InputRulesFiles/Rules8-8-0-to-8-9-0.md