-
Notifications
You must be signed in to change notification settings - Fork 250
Enable AveragedSpecifiedTimes to be used in output writers
#4876
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
5fcbcfa
5457bc5
96872aa
61d29ac
c20c9b0
35c41a5
6d16825
6ea30f5
56e6e55
b3f1541
ed15fd6
0efdbc8
3ab8e0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -91,30 +91,81 @@ function (sch::AveragedTimeInterval)(model) | |||||||||||||||||||||
| return scheduled | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
| initialize_schedule!(sch::AveragedTimeInterval, clock) = nothing | ||||||||||||||||||||||
| outside_window(sch::AveragedTimeInterval, clock) = clock.time <= next_actuation_time(sch) - sch.window | ||||||||||||||||||||||
| outside_window(sch::AveragedTimeInterval, clock) = clock.time <= next_actuation_time(sch) - sch.window | ||||||||||||||||||||||
| end_of_window(sch::AveragedTimeInterval, clock) = clock.time >= next_actuation_time(sch) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| TimeInterval(sch::AveragedTimeInterval) = TimeInterval(sch.interval) | ||||||||||||||||||||||
| Base.copy(sch::AveragedTimeInterval) = AveragedTimeInterval(sch.interval, window=sch.window, stride=sch.stride) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| mutable struct AveragedSpecifiedTimes <: AbstractSchedule | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| A schedule for averaging over windows that precede SpecifiedTimes. | ||||||||||||||||||||||
| """ | ||||||||||||||||||||||
| mutable struct AveragedSpecifiedTimes <: AbstractSchedule | ||||||||||||||||||||||
| mutable struct AveragedSpecifiedTimes{W <: Union{Float64, Vector{Float64}}} <: AbstractSchedule | ||||||||||||||||||||||
| specified_times :: SpecifiedTimes | ||||||||||||||||||||||
|
||||||||||||||||||||||
| window :: Float64 | ||||||||||||||||||||||
| window :: W | ||||||||||||||||||||||
| stride :: Int | ||||||||||||||||||||||
| collecting :: Bool | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const VaryingWindowAveragedSpecifiedTimes = AveragedSpecifiedTimes{Vector{Float64}} | ||||||||||||||||||||||
|
||||||||||||||||||||||
| const VaryingWindowAveragedSpecifiedTimes = AveragedSpecifiedTimes{Vector{Float64}} | |
| const VaryingWindowAveragedSpecifiedTimes = AveragedSpecifiedTimes{<:Vector} |
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 don't think you want to limit to Float64. That won't work with dates.
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.
Should we relax AveragedTimeInterval as well?
Oceananigans.jl/src/OutputWriters/windowed_time_average.jl
Lines 15 to 22 in f07428b
| mutable struct AveragedTimeInterval <: AbstractSchedule | |
| interval :: Float64 | |
| window :: Float64 | |
| stride :: Int | |
| first_actuation_time :: Float64 | |
| actuations :: Int | |
| collecting :: Bool | |
| end |
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.
yes! good catch. I can do that in another PR if you like
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.
in the above, interval and window have a "time period" type, while first_actuation_time has a "time" type. These are different when using dates.
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.
is there currently a TimePeriod type? I can try to define one.
Should we also make a SpecifiedWindows like SpecifiedTimes in the case for AveragedSpecifiedTimes?
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.
We don't want a specific type, because sometimes a "period" is Float64 (the assumption here), but with dates, it may need to be Dates.Second or Dates.Day (for example). However, a "time type" can be Float64 (the assumption here) or DateTime.
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.
should we relax it and make it parametric? or constrain it so that interval :: Union{Float64, Dates.Period}
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.
Right, we need to use type parameters, and there need to be two of them.
We don't use the type union approach because it is not concrete; with that approach types can no longer be inferred.
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.