Skip to content
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

Jump to bottom

log_diag_field_info updates for main #1117

Merged
merged 4 commits into from
Feb 14, 2023
Merged

log_diag_field_info updates for main #1117

merged 4 commits into from
Feb 14, 2023

Conversation

rem1776
Copy link
Contributor

@rem1776 rem1776 commented Jan 25, 2023

Description
This takes the changes from #1090 and adds any applicable changes to main instead of dmUpdate.

  • Changes the log routine to be able to take an alternative separator (default is |) via a nml flag
  • adds the axes list as an argument rather than retrieving the list during the routine.
  • moves header write to first call instead of in diag_manager_init (still opened during init)

How Has This Been Tested?
oneapi 23, amd box

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

thomas-robinson
thomas-robinson requested changes Jan 26, 2023
View reviewed changes
Copy link
Member

@thomas-robinson thomas-robinson left a comment

Choose a reason for hiding this comment

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

You should use error_mesg instead of mpp_error. Also remove any of the modern diag stuff.

diag_manager/diag_manager.F90 Outdated
! </NAMELIST>

USE time_manager_mod, ONLY: set_time, set_date, get_time, time_type, OPERATOR(>=), OPERATOR(>),&
& OPERATOR(<), OPERATOR(==), OPERATOR(/=), OPERATOR(/), OPERATOR(+), ASSIGNMENT(=), get_date, &
& get_ticks_per_second
USE mpp_mod, ONLY: mpp_get_current_pelist, mpp_pe, mpp_npes, mpp_root_pe, mpp_sum

USE mpp_mod, ONLY: input_nml_file
USE mpp_mod, ONLY: input_nml_file, mpp_error
Copy link
Member

Choose a reason for hiding this comment

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

diag manager is using error_mesg from fms_mod. I don't think you should add another error reporting routine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're the same routine, error_mesg doesn't do anything but call mpp_error with the same arguments. We should start getting rid of it where possible to get rid of the redudancy.

diag_manager/diag_manager.F90 Outdated

! Fatal error if the module has not been initialized.
IF ( .NOT.module_is_initialized ) THEN
! <ERROR STATUS="FATAL">diag_manager has NOT been initialized</ERROR>
CALL error_mesg ('diag_manager_mod::register_static_field', 'diag_manager has NOT been initialized', FATAL)
CALL mpp_error(FATAL, 'diag_manager_mod::register_static_field_old. diag_manager has NOT been initialized')
Copy link
Member

Choose a reason for hiding this comment

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

This should to be error_mesg. There's no need to update this error message.

diag_manager/diag_manager.F90 Outdated
@@ -653,8 +657,8 @@ INTEGER FUNCTION register_static_field(module_name, field_name, axes, long_name,
TYPE IS (real(kind=r8_kind))
missing_value_use = real(missing_value)
CLASS DEFAULT
CALL error_mesg ('diag_manager_mod::register_static_field',&
& 'The missing_value is not one of the supported types of real(kind=4) or real(kind=8)', FATAL)
CALL mpp_error (FATAL, 'diag_manager_mod::register_static_field. The missing_value is not one of '// &
Copy link
Member

Choose a reason for hiding this comment

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

This also doesn't need to be updated.

diag_manager/diag_util.F90 Outdated
Comment on lines 53 to 55
!! for modern diag
!time_unit_list, max_files, get_base_year, get_base_month, get_base_day, get_base_hour, get_base_minute,
!get_base_second,
Copy link
Member

Choose a reason for hiding this comment

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

Take this out. It shouldn't be in here.

diag_manager/diag_util.F90 Outdated
Comment on lines 65 to 66
!! for modern diag
!USE fms_diag_time_utils_mod, ONLY: diag_time_inc, get_time_string, get_date_dif
Copy link
Member

Choose a reason for hiding this comment

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

This should also be removed.

diag_manager/diag_util.F90 Outdated
@@ -68,7 +73,7 @@ MODULE diag_util_mod
USE time_manager_mod,ONLY: time_type, OPERATOR(==), OPERATOR(>), NO_CALENDAR, increment_date,&
& increment_time, get_calendar_type, get_date, get_time, leap_year, OPERATOR(-),&
& OPERATOR(<), OPERATOR(>=), OPERATOR(<=), OPERATOR(==)
USE mpp_mod, ONLY: mpp_npes
USE mpp_mod, ONLY: mpp_npes, mpp_error
Copy link
Member

Choose a reason for hiding this comment

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

No need for mpp_error

diag_manager/diag_util.F90 Outdated
WRITE (lmin,*) range_use(1)
WRITE (lmax,*) range_use(2)
CLASS DEFAULT
CALL mpp_error('diag_util_mod::log_diag_field_info',&
Copy link
Member

Choose a reason for hiding this comment

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

Use error_mesg. That's the style of the file.

diag_manager/diag_util.F90 Outdated
IF ( TRIM(axes_list) /= '' ) axes_list = TRIM(axes_list)//','
axes_list = TRIM(axes_list)//TRIM(axis_name)
END DO
if ( .not. wrote_header ) then
Copy link
Member

Choose a reason for hiding this comment

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

Is this thread safe? This is a local saved variable, so won't it happen twice if there are 2 threads?

@rem1776
Copy link
Contributor Author

rem1776 commented Feb 1, 2023

@thomas-robinson this is ready, took out any mpp_error's and moved the header write call back to fix any potential threading issues

@rem1776 rem1776 merged commit f0e8cab into NOAA-GFDL:main Feb 14, 2023
thomas-robinson added a commit that referenced this pull request Feb 22, 2023
@thomas-robinson @ganganoaa @rem1776
Dm merge fix (#1135)
5c6636d 
* fix: pointer to allocatable in amip_interp_type (#1106)

* feat: log_diag_field_info updates from diag rewrite (#1117)

---------

Co-authored-by: ganganoaa <[email protected]>
Co-authored-by: Ryan Mulhall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomas-robinson thomas-robinson thomas-robinson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rem1776 @thomas-robinson