Changes to filename_template checks

[abishekg7]

Issue:

Previously, if two active, output stream definitions in the streams. file specified the same string as the filename_template attribute, the corresponding MPAS CORE would crash with an error message. This was also the case for when one of the streams, sharing the filename_template, included an inactive package, hence effectively making it inactive. The existing logic to perform stream uniqueness checks in src/framework/xml_stream_parser.c did not have the information about which packages associated with a stream were active, and hence did not account for inactive packages.

Solution

This PR introduces a new subroutine MPAS_stream_mgr_check_filename_template in src/framework/mpas_stream_manager.F to check whether two active streams might potentially be accessing the same file (via filename_template) resulting in file conflicts. More specifically, this subroutine checks that there are no active output streams that share an identical filename_template attribute with any other active input or output streams in the stream manager. An active stream in this context is a stream with attribute active_stream = .true. and is not deactivated by an inactive package and is either an input or output stream with at least one active alarm. This routine can be called from within MPAS_stream_mgr_validate_streams or separately as needed.

This PR also introduces a new function 'MPAS_stream_mgr_get_num_alarms', which returns the number of active alarms for a given stream in the specified direction.

The previous logic to check for unique filename_template attributes in src/framework/xml_stream_parser.c has been removed, while retaining the checks for unique stream names in the same location. Note that the previous logic only check for conflicts between two output streams, whereas the new logic also checks for potential conflicts between an input stream and an output stream accessing the same file.

[mgduda]

I've run a few quick tests, and it looks like the core logic is now functioning as expected. I had just a couple of ideas that might clean up the implementation:

1. The outer loop (with stream1_cursor) could cycle on anything other than an active output stream, and that might obviate the need for checks like (stream1_input .and. stream2_input) in the inner loop.
2. If the inner loop (with stream2_cursor) iterated only over streams that occur in the stream list after stream1_cursor, that would eliminate duplicate error messages (e.g., "Found identical values ... for ... foobar_1 and foobar_2" and later "Found identical values ... for ... foobar_2 and foobar_1").

[abishekg7]

I've run a few quick tests, and it looks like the core logic is now functioning as expected. I had just a couple of ideas that might clean up the implementation:

1. The outer loop (with stream1_cursor) could cycle on anything other than an active output stream, and that might obviate the need for checks like (stream1_input .and. stream2_input) in the inner loop.
2. If the inner loop (with stream2_cursor) iterated only over streams that occur in the stream list after stream1_cursor, that would eliminate duplicate error messages (e.g., "Found identical values ... for ... foobar_1 and foobar_2" and later "Found identical values ... for ... foobar_2 and foobar_1").

I've incorporated the 2nd idea in the most recent commit. But having either the outer loop or the inner loop consider only output streams, appears to miss some filename conflicts, due to the directionality introduced by idea #2.

[mgduda]

The error message can be quite long when printed to a single line, e.g.,

ERROR: Found identical values of the filename_template attribute for multiple active output streams, restart and output, in streams.<CORE>. This may result in file conflicts.

It might be better to explicitly write a multi-line error message with multiple calls to mpas_log_write.

[mgduda]

@abishekg7 It looks like this suggestion is still pending.

[abishekg7]

The previous message has actually been modified slightly, and it is split up into two calls to mpas_log_write. Let me know if it needs further changes.

[abishekg7]

I have addressed the remaining suggestions in the last few commits.

[mgduda]

@abishekg7 As of now, there are quite a few messages that are written to the log file even when there are no issues with the streams. For example, here's what I get from the log file for the atmosphere core:

 Stream 1 invariant filename invariant.nc  active = T input = F output = F  packages_active T
 Stream 1 input filename x1.40962.init.nc  active = T input = T output = F  packages_active T
 Stream 2 restart filename restart.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: T
 Stream 2 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F pkg_active: F
 Stream 2 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: F
 Stream 2 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 restart filename restart.**-**-**_**.**.**.nc  active = T input = T output = T  packages_active T
 Stream 2 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F pkg_active: F
 Stream 2 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: F
 Stream 2 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F  packages_active F
 Stream 1 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F  packages_active F
 Stream 1 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F  packages_active F
 Stream 1 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F  packages_active F
 Stream 1 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T  packages_active F
 Stream 1 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 surface filename x1.40962.sfc_update.nc  active = T input = F output = F  packages_active T
 Stream 1 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active F

Could you update the PR so that these messages are not written?

[abishekg7]

@abishekg7 As of now, there are quite a few messages that are written to the log file even when there are no issues with the streams. For example, here's what I get from the log file for the atmosphere core:

 Stream 1 invariant filename invariant.nc  active = T input = F output = F  packages_active T
 Stream 1 input filename x1.40962.init.nc  active = T input = T output = F  packages_active T
 Stream 2 restart filename restart.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: T
 Stream 2 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F pkg_active: F
 Stream 2 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: F
 Stream 2 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 restart filename restart.**-**-**_**.**.**.nc  active = T input = T output = T  packages_active T
 Stream 2 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F pkg_active: F
 Stream 2 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F pkg_active: F
 Stream 2 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T pkg_active: F
 Stream 2 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 iau filename x1.40962.AmB.**-**-**_**.**.**.nc  active = T input = T output = F  packages_active F
 Stream 1 lbc_in filename lbc.**-**-**_**.**.**.nc  active = T input = T output = F  packages_active F
 Stream 1 ugwp_oro_data_in filename x1.40962.ugwp_oro_data.nc  active = T input = T output = F  packages_active F
 Stream 1 ugwp_ngw_in filename ugwp_limb_tau.nc  active = T input = T output = F  packages_active F
 Stream 1 da_state filename mpasout.**-**-**_**.**.**.nc  active = T input = T output = T  packages_active F
 Stream 1 output filename history.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active T
 Stream 2 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 diagnostics filename diag.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active T
 Stream 2 surface filename x1.40962.sfc_update.nc  active = T input = F output = F pkg_active: T
 Stream 2 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 1 surface filename x1.40962.sfc_update.nc  active = T input = F output = F  packages_active T
 Stream 1 diag_ugwp filename diag_ugwp.**-**-**_**.**.**.nc  active = T input = F output = T  packages_active F

Could you update the PR so that these messages are not written?

Done.

[mgduda]

I wonder if we might be better off omitting these messages from the log file. As they are now, they might not be particularly valuable to most users. Here's what I'm seeing in a run of the init_atmosphere core:

 Stream 2 output filename init.nc  active = T input = F output = T pkg_active: T
 Stream 2 ugwp_oro_data filename ugwp_oro_data.nc  active = T input = F output = T pkg_active: F
 Stream 2 surface filename sfc_update.nc  active = T input = F output = T pkg_active: F
 Stream 2 lbc filename lbc.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F
 Stream 2 ugwp_oro_data filename ugwp_oro_data.nc  active = T input = F output = T pkg_active: F
 Stream 2 surface filename sfc_update.nc  active = T input = F output = T pkg_active: F
 Stream 2 lbc filename lbc.**-**-**_**.**.**.nc  active = T input = F output = T pkg_active: F

[abishekg7]

I left this in for debugging purposes until this PR reaches the approval stage. Can remove it if there are no other changes to the main logic required.

[mgduda]

It's possible that the logic may still need some adjustment. When I have the following two streams defined

<immutable_stream name="restart"
                  type="input;output"
                  filename_template="restart.$Y-$M-$D_$h.$m.$s.nc"
                  input_interval="initial_only"
                  output_interval="6:00:00" />

<stream name="foo"
        type="input"
        filename_template="restart.$Y-$M-$D_$h.$m.$s.nc"
        input_interval="6:00:00" >

        <file name="stream_list.atmosphere.output"/>
</stream>

I don't get any runtime errors, though I would have expected there to be since the restart stream is an input;output stream and it shares a filename_template with the foo stream.

[abishekg7]

Fixed in 77c20a3