-
Notifications
You must be signed in to change notification settings - Fork 8
Use datetime to control time #667
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?
Conversation
|
@SeanBryan51 The head of the branch now compiles in serial mode. I added a few functions to the |
|
Thanks @Whyborn, this is exciting! I should be able to take a closer look at this on Monday |
…' into 656-use-datetime-to-control-time
|
@SeanBryan51 I pulled the calendar checking into |
|
Benchcab run shows bitwise compatibility |
SeanBryan51
left a comment
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.
Thanks for this @Whyborn
I think it's worth trying out a CASA-CNP run, although we don't have a standard benchcab configuration for it. I've been doing my MPI-serial comparisons using this repo which is based off the payu crujra spatial configuration. It was pretty straight forward to adapt the base configuration to run a CASA (cold start) run, see this diff: SeanBryan51/crujra_accessN96_1h@main...casa-cnp-configuration
I'm happy to test this out with a CASA run using my scripts if you like?
| @@ -0,0 +1,1496 @@ | |||
| module datetime_module | |||
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.
Always best to include a copy of the copyright and permission notice from the original repo (https://github.com/wavebitscientific/datetime-fortran/blob/main/LICENSE):
| module datetime_module | |
| ! Copyright (c) 2013-2020 datetime-fortran contributors | |
| ! | |
| ! Permission is hereby granted, free of charge, to any person obtaining a copy | |
| ! of this software and associated documentation files (the "Software"), to deal | |
| ! in the Software without restriction, including without limitation the rights | |
| ! to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | |
| ! copies of the Software, and to permit persons to whom the Software is | |
| ! furnished to do so, subject to the following conditions: | |
| ! | |
| ! The above copyright notice and this permission notice shall be included in all | |
| ! copies or substantial portions of the Software. | |
| ! | |
| ! THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | |
| ! IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | |
| ! FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | |
| ! AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | |
| ! LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | |
| ! OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE | |
| ! SOFTWARE. | |
| module datetime_module |
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.
Yep for sure
| pure elemental logical function isNewDay(d) | ||
| ! Determines whether the given `datetime` `d` is a the start of a month. | ||
| type(datetime), intent(in) :: d | ||
|
|
||
| isNewDay = (d%getHour() == 0 .and. d%getMinute() == 0 .and.& | ||
| d%getSecond() == 0 .and. d%getMillisecond() == 0) | ||
|
|
||
| end function isNewDay | ||
|
|
||
|
|
||
| pure elemental logical function isNewMonth(d) | ||
| ! Determines whether the given `datetime` `d` is the start of a month. | ||
| type(datetime), intent(in) :: d | ||
|
|
||
| isNewMonth = (d%getDay() == 1 .and. d%getHour() == 0 .and.& | ||
| d%getMinute() == 0 .and. d%getSecond() == 0 .and.& | ||
| d%getMillisecond() == 0) | ||
| end function isNewMonth | ||
|
|
||
|
|
||
| pure elemental logical function isNewYear(d) | ||
| ! Determines whether the given `datetime` `d` is the start of a year. | ||
| type(datetime), intent(in) :: d | ||
|
|
||
| isNewYear = (d%getMonth() == 1 .and. d%getDay() == 1 .and.& | ||
| d%getHour() == 0 .and. d%getMinute() == 0 .and.& | ||
| d%getSecond() == 0 .and. d%getMillisecond() == 0) | ||
| end function isNewYear |
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.
Are we sure that the datetime argument will be set such that the hour, minute, second and millisecond values are all zero? If the datetime argument is dependent on the time step then it might not line up exactly.
JULES uses this code which checks if going back a time step causes the current year to change which like seems like a clean way to do it
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 would do, since they're integer values, you'll never get round-off error throwing off the counter, assuming that you check on initialisation that the time-step is a valid value that divides evenly into a day.
That JULES way is neater in some situations, but it does require two bits of information rather than 1, which could be more annoying passing it down the call stack. If I was to adopt a similar method I'd rather have routines like is_end_of_day, is_start_of_day, rather than advancing/retreating the timestep by 1 interval, so something like
curr_time = <some_datetime>
dt = <some_timedelta>
is_start_of_day(curr_time, dt) ! Would check the day of curr_time, and curr_time - dt
is_end_of_day(curr_time, dt) ! Would check the day of curr_time, and curr_time + dt
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.
The time step can be pretty large ~3 hours for some configurations, so advancing the datetime object by a time step might not hit hour 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 think it's a requirement already (implicitly if not explicitly) that timesteps must fit evenly into a day? Otherwise I think the current methods of checking would fail/be inconsistent
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.
You might be right, I'm not too familiar with how the time step is set.
Can you confirm what the valid time steps are? I'm also wondering about coupled model and what they use for the time step?
|
I'll give a go with the CASA-CNP configuration- I should be able to just clone the branch from your repo right? |
|
Unfortunately getting entirely unhelpful MPI errors when running the branch with MPI. Might have to run it through a debugger |
CABLE
Thank you for submitting a pull request to the CABLE Project.
Description
Testing using the
fortran-datetimepackage, with some augmentations, to control time and triggering of events in CABLE.📚 Documentation preview 📚: https://cable--667.org.readthedocs.build/en/667/