Skip to content

Changes to filename_template checks #1464

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.

Sign up for GitHub

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

Draft
abishekg7 wants to merge 15 commits into
base: develop
Choose a base branch
from
+172 −7
Draft

Changes to filename_template checks #1464

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
30b74f7
first draft
abishekg7 Jun 2, 2026
42399bc
Only crash out if it's an output stream
abishekg7 Jun 3, 2026
74af38b
Rename subroutine and call from stream_mgr_validate_streams
abishekg7 Jun 3, 2026
8eb3e56
Remove commented lines
abishekg7 Jun 3, 2026
64e5856
Some cleanups addressing review comments
abishekg7 Jun 4, 2026
3bf5404
remove trailing whitespace
abishekg7 Jun 4, 2026
5c7a075
Negating if conditions, removing unused variables and whitespace fixes
abishekg7 Jun 4, 2026
97c6d25
Edits to pass ierr to calling routine
abishekg7 Jun 4, 2026
741c495
Fixes to address review comments, and cleanup of logging statements
abishekg7 Jun 5, 2026
035a4a7
Redoing logic
abishekg7 Jun 10, 2026
f5e7f5c
Using stream_mgr_get_num_alarms in the logic. And some cleanup
abishekg7 Jun 11, 2026
ff95f35
Modifying logic so inner loop starts with the outer loop iterator
abishekg7 Jun 12, 2026
5d54033
Make ierr in get_num_alarms optional and cleanup unused variables
abishekg7 Jun 15, 2026
a30e46c
Fixing whitespace, comment block and error messages
abishekg7 Jun 16, 2026
0376fed
Reducing number of variables + cleanup
abishekg7 Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 161 additions & 0 deletions src/framework/mpas_stream_manager.F
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,126 @@ subroutine MPAS_stream_mgr_validate_streams(manager, streamID, ierr)!{{{
#endif

end do

! Only check filename_template uniqueness when validating all streams
if (.not. present(streamID)) then
call MPAS_stream_mgr_check_filename_template(manager, ierr=err_local)
Comment thread
mgduda marked this conversation as resolved.
if (present(ierr)) ierr = err_local
end if

end subroutine MPAS_stream_mgr_validate_streams!}}}


!-----------------------------------------------------------------------
! routine MPAS_stream_mgr_check_filename_template
!
!> \brief Check for identical filename templates in active I/O streams.
!> \author Abishek Gopal
!> \date June 3 2026
!> \details
!> Checks that there are no identical filename templates among active I/O
!> streams in the stream manager, which may lead to file conflicts. This
!> routine can be called from within MPAS_stream_mgr_validate_streams or
!> separately as needed.

@mgduda mgduda Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we add a sentence or two about the return values from this routine? That may be especially relevant, since the return value of this routine may be used directly as the return value to MPAS_stream_mgr_validate_streams.

It might also be helpful to add a bit more detail as to what exactly this routine is checking. For example, the description "among active I/O streams" suggests that two active input streams might be caught by this routine.

!
!-----------------------------------------------------------------------
subroutine MPAS_stream_mgr_check_filename_template(manager, ierr)!{{{

implicit none

type (MPAS_streamManager_type), intent(inout) :: manager
integer, intent(out), optional :: ierr
Comment thread
mgduda marked this conversation as resolved.

integer :: threadNum, err_local,err_local1, err_local2

@mgduda mgduda Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you take another pass through the code changes in this PR and regularize the whitespace? For example, there's no space between err_local and err_local1 here. Other places, the whitespace is inconsistent within parentheses (e.g., lines 608 and 612).

character (len=StrKIND) :: message, streamID
type (MPAS_stream_list_type), pointer :: stream1_cursor, stream2_cursor
type (MPAS_TimeInterval_type) :: clock_interval
logical :: stream1_pkg_active, stream2_pkg_active
logical :: stream1_input, stream2_input
logical :: stream1_output, stream2_output
integer :: stream1_input_nalarms, stream1_output_nalarms
integer :: stream2_input_nalarms, stream2_output_nalarms

STREAM_DEBUG_WRITE('-- Called MPAS_stream_mgr_check_filename_template() for all streams')

if (present(ierr)) ierr = MPAS_STREAM_MGR_NOERR

threadNum = mpas_threading_get_thread_num()

if ( threadNum == 0 ) then

streamID = '.*' ! query all streams

nullify(stream1_cursor)
do while (MPAS_stream_list_query(manager % streams, streamID, stream1_cursor))
Comment thread
mgduda marked this conversation as resolved.

stream1_pkg_active = stream_active_pkg_check(stream1_cursor)

stream1_input_nalarms = MPAS_stream_mgr_get_num_alarms(manager, trim(stream1_cursor % name), &
MPAS_STREAM_INPUT, err_local1)
stream1_output_nalarms = MPAS_stream_mgr_get_num_alarms(manager, trim(stream1_cursor % name), &
MPAS_STREAM_OUTPUT, err_local2)

@mgduda mgduda Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't look like the err_local1 or err_local2 return error codes are ever used. It might simplify the code to make ierr an optional argument to MPAS_stream_mgr_get_num_alarms, and to not pass any corresponding actual argument in these calls. That would also allow the local variables err_local1 and err_local2 to be removed.


stream1_input = ((stream1_cursor % direction == MPAS_STREAM_INPUT .or. &
stream1_cursor % direction == MPAS_STREAM_INPUT_OUTPUT) .and. &
stream1_input_nalarms > 0)
stream1_output = ((stream1_cursor % direction == MPAS_STREAM_OUTPUT .or. &
stream1_cursor % direction == MPAS_STREAM_INPUT_OUTPUT) .and. &
stream1_output_nalarms > 0)

@mgduda mgduda Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I may be mistaken, but with the exception of stream1_input, the above stream1_* variables are only used until the point where we enter the second loop over streams, and so it might be possible to simply declare the variables pkg_active (rather than stream1_pkg_active and stream2_pkg_active), etc. This would cut down on the number of local variables that the reader would need to consider when looking through this code.


call mpas_log_write('Stream 1 '//trim(stream1_cursor % name)//' filename '//trim(stream1_cursor % filename_template)//' &
& active = $l input = $l output = $l packages_active $l',logicArgs=(/stream1_cursor % active_stream, stream1_input, stream1_output, stream1_pkg_active/))

if (.not. stream1_cursor % active_stream &
.or. .not. (stream1_input .or. stream1_output) &
.or. .not. stream1_pkg_active) then
Comment thread
mgduda marked this conversation as resolved.
Outdated
cycle
end if

nullify(stream2_cursor)
stream2_cursor => stream1_cursor
do while (MPAS_stream_list_query(manager % streams, streamID, stream2_cursor))

stream2_input_nalarms = MPAS_stream_mgr_get_num_alarms(manager, trim(stream2_cursor % name), &
MPAS_STREAM_INPUT, err_local1)
stream2_output_nalarms = MPAS_stream_mgr_get_num_alarms(manager, trim(stream2_cursor % name), &
MPAS_STREAM_OUTPUT, err_local2)

stream2_pkg_active = stream_active_pkg_check(stream2_cursor)
stream2_input = ((stream2_cursor % direction == MPAS_STREAM_INPUT .or. &
stream2_cursor % direction == MPAS_STREAM_INPUT_OUTPUT) .and. &
stream2_input_nalarms > 0)
stream2_output = ((stream2_cursor % direction == MPAS_STREAM_OUTPUT .or. &
stream2_cursor % direction == MPAS_STREAM_INPUT_OUTPUT) .and. &
stream2_output_nalarms > 0)

call mpas_log_write('Stream 2 '//trim(stream2_cursor % name)//' filename '//trim(stream2_cursor % filename_template)//' &
& active = $l input = $l output = $l pkg_active: $l',logicArgs=(/stream2_cursor % active_stream, stream2_input, stream2_output, stream2_pkg_active/))

if (.not. stream2_cursor % active_stream & ! Skip if Stream 2 is not active
.or. .not. (stream2_input .or. stream2_output) &
.or. (stream1_input .and. stream2_input) & ! Skip if Streams 1 and 2 are both input streams
.or. .not. stream2_pkg_active) then ! Skip if Stream 2 is inactive due to inactive package
cycle
end if

if (trim(stream1_cursor % filename_template) == trim(stream2_cursor % filename_template)) then
message = 'Found identical values of the filename_template attribute for multiple active output streams, ' &
// trim(stream1_cursor % name) // ' and ' // trim(stream2_cursor % name) // &
', in streams.<CORE>. This may result in file conflicts.'
call mpas_log_write(message, messageType=MPAS_LOG_ERR)

@mgduda mgduda Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

if (present(ierr)) ierr = MPAS_STREAM_MGR_ERROR
end if

end do

end do

end if

end subroutine MPAS_stream_mgr_check_filename_template!}}}


!-----------------------------------------------------------------------
! routine MPAS_stream_mgr_destroy_stream
!
Expand Down Expand Up @@ -1771,6 +1887,51 @@ function MPAS_stream_mgr_get_stream_interval(manager, streamID, direction, ierr)
end function MPAS_stream_mgr_get_stream_interval


!-----------------------------------------------------------------------
! routine MPAS_stream_mgr_get_num_alarms
!
!> \brief Returns the number of I/O alarms associated with a stream in a given direction
!> \author Abishek Gopal
!> \date 10 June 2026
!> \details
!> Returns the number of I/O alarms associated with a stream in a given direction.
!
!-----------------------------------------------------------------------
function MPAS_stream_mgr_get_num_alarms(manager, streamID, direction, ierr) result(numAlarms)

implicit none

integer :: numAlarms

type (MPAS_streamManager_type), intent(in) :: manager
character(len=*), intent(in) :: streamID
integer, intent(in) :: direction
integer, intent(out) :: ierr

type (mpas_stream_list_type), pointer :: streamCursor

numAlarms = 0
ierr = MPAS_STREAM_MGR_NOERR
nullify(streamCursor)

if ( mpas_stream_list_query(manager % streams, streamID, streamCursor) ) then
if (direction == MPAS_STREAM_INPUT) then
numAlarms = MPAS_stream_list_length(streamCursor % alarmList_in, ierr)
else if (direction == MPAS_STREAM_OUTPUT) then
numAlarms = MPAS_stream_list_length(streamCursor % alarmList_out, ierr)
else
ierr = MPAS_STREAM_MGR_ERROR
call MPAS_stream_mesg(manager % errorLevel, 'Invalid direction encountered in MPAS_stream_mgr_get_num_alarms')
end if
else
ierr = MPAS_STREAM_MGR_ERROR
call MPAS_stream_mesg(manager % errorLevel, 'Stream ''' // trim(streamID) // ''' &
does not exist. Cannot retrieve number of alarms in stream.')
end if

end function MPAS_stream_mgr_get_num_alarms


!-----------------------------------------------------------------------
! routine MPAS_stream_mgr_set_property_int
!
Expand Down
7 changes: 0 additions & 7 deletions src/framework/xml_stream_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,6 @@ int uniqueness_check(ezxml_t stream1, ezxml_t stream2)
fmt_err(msgbuf);
return 1;
}
if (strstr(type, "output") != NULL || strstr(type2, "output") != NULL){
if (strcmp(filename, filename2) == 0) {
snprintf(msgbuf, MSGSIZE, "Output streams \"%s\" and \"%s\" cannot share the filename_template \"%s\".", name, name2, filename);
fmt_err(msgbuf);
return 1;
}
}
}

return 0;
Expand Down