Skip to content
This repository has been archived by the owner on Oct 23, 2020. It is now read-only.

Apply stream reference_time to individual output records #1418

Conversation

matthewhoffman
Copy link
Member

@matthewhoffman matthewhoffman commented Sep 25, 2017

Previously, a stream's reference_time only applied to determining the
filename breaks. Right now, output is always relative to the time of
the start of the current simulation (even if it is a restart). There is
no way to force, say, monthly output (i.e. on the 1st of every month) if
your restart does not start on the first of a month.

This commit extends reference_time to also apply to
determining when individual records should be written. Thus, output
stream behavior will not change after a restart.

This is done by adding a new routine to mpas_timekeeping that adjusts
the prevRingTime of an alarm to be an integer multiple of the stream's
output_interval away from the stream's reference_time. This routine is
called for each output stream created by the stream manager.

Fixes Issue #1307.

Previously, a stream's reference_time only applied to determining the
filename breaks.  Right now, output is always relative to the time of
the start of the current simulation (even it it is a restart). There is
no way to force, say, monthly output (i.e. on the 1st of every month) if
your restart does not start on the first of a month.

This commit extends reference_time to also apply to
determing when individual records should be written.  Thus, output
stream behavior will not change after a restart.

This is done by adding a new routine to mpas_timekeeping that adjusts
the prevRingTime of an alarm to be an integer multiple of the stream's
output_interval away from the stream's reference_time.  This routine is
called for each output stream created by the stream manager.
@matthewhoffman
Copy link
Member Author

To test, I set up a restart test that does a cold start followed by a restart.
The cold start has the following namelist and streams settings. The key thing is to make the output and restart intervals indivisible (here using 5 and 7 days).

    config_dt = '0000-00-01_00:00:00'
    config_do_restart = .false.
    config_start_time = 0000-01-01_00:00:00
    config_stop_time = '0000-01-08_00:00:00'
<immutable_stream name="restart"
                  filename_interval="output_interval"
                  clobber_mode="truncate"
                  reference_time="0000-01-01_00:00:00"
                  precision="double"
                  output_interval="0000-00-07_00:00:00"
                  filename_template="rst.$Y.$M.$D.nc"
                  input_interval="initial_only"
                  type="input;output"
                  input_interal="initial_only"/>

<stream name="output"
        clobber_mode="overwrite"
        reference_time="0000-01-01_00:00:00"
        output_interval="0000-00-05_00:00:00"
        filename_template="output.nc"
        type="output">
	<var name="xtime"/>
</stream>

The restart is identical except for:

    config_do_restart = .true.
    config_start_time = 'file'
    config_stop_time = '0000-01-20_00:00:00'

With this configuration the output file contains:

 xtime =
  "0000-01-01_00:00:00                                             ",
  "0000-01-06_00:00:00                                             ",
  "0000-01-13_00:00:00                                             ",
  "0000-01-18_00:00:00                                             " ;

This is incorrect because we asked for output to occur every 5 days, so we should have gotten it on day 11 and 16 instead of day 13 and 18 - the restart on day 8 threw it off.

Rerunning the test with this branch, the output is correct:

 xtime =
  "0000-01-01_00:00:00                                             ",
  "0000-01-06_00:00:00                                             ",
  "0000-01-11_00:00:00                                             ",
  "0000-01-16_00:00:00                                             " ;

I tried a second test where the stream reference_time is in the future, but I get an unrelated error that exists prior to this branch:

At line 1096 of file mpas_timekeeping.F
Fortran runtime error: Bad integer for item 1 in list input

We could try to fix this in this branch if anyone is willing to figure it out. I took a quick look but was unable to get a usable backtrace.

Note that I included code for reverse-running clocks but I did not test it.

! Local variables
type (MPAS_Alarm_type), pointer :: alarmPtr
type (MPAS_TimeInterval_type) :: searchInterval, searchRemainder
integer (kind=I8KIND) :: nDivs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matthewhoffman I added this I8 kind in an extra commit to get it to compile.

Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! I was able to mimic your exact test, with success, when I merged this into ocean/develop. Thank you @matthewhoffman, that is one more annoying item off our list. This is a bit like swatting flies.

@mark-petersen
Copy link
Contributor

An important note for ACME is that the restart and all output streams should have
reference_time="0000-01-01_00:00:00"
added to it to get this to work. @wolfe, I made a note of that.

@mark-petersen
Copy link
Contributor

@matthewhoffman I'm ready to take off in progress and don't merge if you are.

@mark-petersen
Copy link
Contributor

@mgduda do you have time to look at this today? We are making a big push to merge these framework PRs and update framework in all the ACME cores.

@mark-petersen
Copy link
Contributor

I ran a test on the original issue, which is monthly output and sub-monthly restarts. I ran the ocean core for 70 days with 10 day restarts, then continue for another 70 days with ten day restarts. Output file is monthly. It worked perfectly.

head of ocean/develop

 xtime =
  "0001-01-01_00:00:00                                             ",
  "0001-02-01_00:00:00                                             ",
  "0001-03-01_00:00:00                                             ",
  "0001-03-12_00:00:00                                             ",
  "0001-04-12_00:00:00                                             ",
  "0001-05-12_00:00:00                                             " ;

This PR merged into ocean/develop

 xtime =
  "0001-01-01_00:00:00                                             ",
  "0001-02-01_00:00:00                                             ",
  "0001-03-01_00:00:00                                             ",
  "0001-04-01_00:00:00                                             ",
  "0001-05-01_00:00:00                                             " ;

Note you still need
config_write_output_on_startup = .false
or you will get an extra entry on restart, which is expected.

For my reference, runs at:

/lustre/scratch3/turquoise/mpeterse/runs/t51p/ocean/global_ocean/QU480/default/forward
/lustre/scratch3/turquoise/mpeterse/runs/t51q/ocean/global_ocean/QU480/default/forward

@mark-petersen
Copy link
Contributor

@mgduda I know you are busy. Any chance I can merge this in today? This is the last framework PR we need, then we can update all our cores and test.

@mark-petersen
Copy link
Contributor

@mgduda Sorry to jump ahead on this, but we have a big ACME code update with framework updates today. This PR worked perfectly in my testing, and solves a problem that has been a long-standing annoyance in ACME. I merged locally and test compiled the atmosphere core with gnu and intel.

@mark-petersen mark-petersen merged commit 5bf84f9 into MPAS-Dev:develop Oct 10, 2017
mark-petersen added a commit that referenced this pull request Oct 10, 2017
…' into develop

Previously, a stream's reference_time only applied to determining the
filename breaks. Right now, output is always relative to the time of
the start of the current simulation (even if it is a restart). There is
no way to force, say, monthly output (i.e. on the 1st of every month) if
your restart does not start on the first of a month.

This commit extends reference_time to also apply to
determining when individual records should be written. Thus, output
stream behavior will not change after a restart.

This is done by adding a new routine to mpas_timekeeping that adjusts
the prevRingTime of an alarm to be an integer multiple of the stream's
output_interval away from the stream's reference_time. This routine is
called for each output stream created by the stream manager.

Fixes Issue #1307.
@mark-petersen mark-petersen deleted the framework/output_record_reference_time branch October 10, 2017 15:28
@matthewhoffman
Copy link
Member Author

@mark-petersen , thanks for sponsoring this PR. BTW, regarding the comment you made about still needing config_write_output_on_startup = .false, you could consider if a change like this:
eaa703f
would be appropriate for the ocean core to eliminate that.

matthewhoffman added a commit that referenced this pull request Oct 10, 2017
Update needed for ACME

Framework features brought in:

* f444d0f Merge PR #1418 'matthewhoffman/framework/output_record_reference_time' into develop
* 263e14f Merge PR #1428 'mark-petersen/framework/couple_fixes' into develop
* bcce31d Merge PR #1424 'amametjanov:az/tools/cp-prebuilt-tools' into develop
* 98cfeea Merge PR# 1349 'akturner/framework/forcing_cleanup' into develop
* 9359319 Merge PR #1347 'akturner/framework/forcing_restart_timestamp' into develop
* e9ce203 Merge PR #1348 'akturner/framework/forcing_at_init' into develop
* 4974284 Merge PR #1368 'akturner/framework/improved_messages_in_driver' into develop
* 86d50c5 Merge PR #1417 'akturner/framework/forcing_multiple_blocks' into develop
* 9116da3 Merge branch 'framework/validation-of-streams-using-interval_in-interval_out' into develop
* e466b46 Merge branch 'framework/interval_in-interval_out-support-for-streams' into develop
* 30dc955 Merge branch 'az/framework/mpas_dmpar-race-fix' into develop
* b632938 Merge branch 'framework/i8_interval_division' into develop
* 6dac06c Merge branch 'framework/log_write_IBM_error' into develop
* 960a648 Merge branch 'framework/cleanup-logging-stream-manager' into develop
* 504c282 Merge branch 'framework/make-streams-with-direction-none-inactive' into develop
* 5903748 Merge branch 'framework/correctly_remove_blk_fields' into develop
* 3565965 Merge branch 'framework/iostreams-real4dfield-bug' into develop
* 8b60591 Merge branch 'framework/missing-deallocate-nEdgesOnCellField-bootstrapping' into develop
* 70b953b Merge branch 'master' into develop
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants