Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adds
last_Δt
toClock
#3508Adds
last_Δt
toClock
#3508Changes from all commits
ac59cdf
dfd62df
99aacbb
a68c34a
51f792b
1852d42
a83e3bb
40db431
65068f6
d5f4916
bc3ca25
689c56c
0170cf0
9820fed
28cc105
b7d3253
a48cbb8
84c206b
5af5f82
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Personally I would add a constructor for
Clock
that takes one type rather than writing this. But you can decide.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.
Also note that we might reasonably expect users to provide custom clocks with their own time and iteration --- but that the stage and time-step probably won't commonly be provided when building a clock. So I think it would make more sense to have a syntax that looks 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.
I think the type of the time-step can probably be inferred from the time type
TT
(ie if the time type is either a floating point or a DateTime or something like that).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.
Yeah that makes sense
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.
There actually is already a constructor I can just change it a bit to infer the
DT
Oceananigans.jl/src/TimeSteppers/clock.jl
Line 27 in 3bb62a6
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 seems like
Dates
only stores things as integers and is never directly used but the timestep is alwayseltype(grid)
so I'm not sure it can be directly infered.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 support dates right now -- to support it, we have to generalize how the time-step specification works.
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 will have to change how time-step specification works to support DateTime properly.