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.

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

Subaxis setup #1056

Merged
merged 27 commits into from
Nov 8, 2022
Merged

Conversation

uramirez8707
Copy link
Contributor

Description

  • Setups the subaxis for the latlon and index cases
  • Finishes setting up the write_axis_data and write_axis_metadata routines

Fixes # (issue)

How Has This Been Tested?
CI

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

endif
diag_axis => this
type is (fmsDiagSubAxis_type)
sub_axis_name = this%subaxis_name//"_sub01"
Copy link
Member

Choose a reason for hiding this comment

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

Why is this "01"? Should these be numbered?

Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this just added to the subaxis name? It gets used in the write too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "_01" is the convention from the previous diag manager, and PP tools expect it to have it.

but I am going to just add it to this%subaxis_name, not sure what i was thinking

diag_manager/fms_diag_axis_object.F90 Show resolved Hide resolved
lat(2) = maxval(corners(:,2))
end select

if (is_cube_sphere) then
Copy link
Member

Choose a reason for hiding this comment

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

Can you label the ifs and dos in this routine? There's a lot of them and they are large.

@@ -13,7 +13,7 @@ module fms_diag_field_object_mod
use diag_data_mod, only: max_field_attributes, fmsDiagAttribute_type
use diag_data_mod, only: diag_null, diag_not_found, diag_not_registered, diag_registered_id, &
&DIAG_FIELD_NOT_FOUND
use mpp_mod, only: fatal, note, warning, mpp_error, mpp_pe, mpp_root_pe
use mpp_mod, only: fatal, note, warning, mpp_error, stdout, mpp_pe, mpp_root_pe
Copy link
Member

Choose a reason for hiding this comment

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

You added stdout but I don't think it's used.

use mpp_mod, only: mpp_get_current_pelist, mpp_npes, mpp_root_pe, mpp_pe, mpp_error, FATAL, stdout
use time_manager_mod, only: time_type, operator(/=), operator(==), date_to_string
Copy link
Member

Choose a reason for hiding this comment

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

This use statement is very similar to one above. Can you either combine it or move it with the other use time_manager_mod

@@ -613,9 +642,11 @@ subroutine dump_file_obj(this, unit_num)
end subroutine

!< @brief Opens the diag_file if it is time to do so
subroutine open_diag_file(this, time_step)
subroutine open_diag_file(this, time_step, file_is_opened)
Copy link
Member

Choose a reason for hiding this comment

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

They're called history files, not diag files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol we don't call them history files anywhere in the code ...

Copy link
Member

Choose a reason for hiding this comment

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

touche

end subroutine open_diag_file

!< @brief Writes the axis metadata for the file
subroutine write_metadata(this, diag_axis)
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 specific for an axis? If it is, it should be called write_axis_metadata

Comment on lines +841 to +842
!TODO: closing the file here for now, just to see if it works
call close_file(fileobj)
Copy link
Member

Choose a reason for hiding this comment

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

oh boy...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gets cleaned up in the next PR, once all of the time stuff is set up "correctly"

@@ -211,15 +211,15 @@ integer function fms_register_diag_field_obj &
fileptr => this%FMS_diag_files(file_ids(i))%FMS_diag_file
call fileptr%add_field_id(fieldptr%get_id())
call fileptr%set_file_domain(fieldptr%get_domain(), fieldptr%get_type_of_domain())
call fileptr%add_axes(axes)
call fileptr%add_axes(axes, this%diag_axis, this%registered_axis)
Copy link
Member

Choose a reason for hiding this comment

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

Is axes the id of the axis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

axes is an array of ids of the axis

@@ -426,9 +427,16 @@ subroutine fms_diag_send_complete(this, time_step)
#else
class(fmsDiagFileContainer_type), pointer :: diag_file !< Pointer to this%FMS_diag_files(i) (for convenience)

logical :: file_is_opened !< True if the file was opened in this time_step
Copy link
Member

Choose a reason for hiding this comment

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

This variable should be named something like file_opened_this_time_step so it's more descriptive.

@uramirez8707
Copy link
Contributor Author

@thomas-robinson I think this is ready for another round of review..

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.

Clean up the use statement, then let me know when it's done and I will merge it.

@@ -29,14 +29,14 @@ module fms_diag_file_object_mod
get_instance_filename, open_file, close_file, get_mosaic_tile_file
use diag_data_mod, only: DIAG_NULL, NO_DOMAIN, max_axes, SUB_REGIONAL, get_base_time, DIAG_NOT_REGISTERED, &
TWO_D_DOMAIN, UG_DOMAIN, prepend_date, DIAG_DAYS, VERY_LARGE_FILE_FREQ
use time_manager_mod, only: time_type, operator(>), operator(/=), operator(==), get_date
use time_manager_mod, only: time_type, operator(>), operator(/=), operator(==), get_date, &
operator(/=), operator(==), date_to_string
Copy link
Member

Choose a reason for hiding this comment

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

you have the operator(/=), operator(==) twice in this use statement

@thomas-robinson thomas-robinson merged commit 1e3e0c0 into NOAA-GFDL:dmUpdate Nov 8, 2022
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
* Adds fms_diag_time_utils.F90 and copies get_time_string and diag_time_inc

* Use diag_time_inc in the fileobj

* finish setting up the io to open the file

* Adds documentation, non-domain decomposed files now only get written by the root PE

* set up the time for the next open and some bug fixes

* Adds code to setups the subaxis

* Fix bugs in the code that get the starting and ending index of the subregion

* fixes bugs in the code that sets up the subaxis object, updates the io for the axis, adds subroutines that dumps the contents of each of the objects

Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
* Adds fms_diag_time_utils.F90 and copies get_time_string and diag_time_inc

* Use diag_time_inc in the fileobj

* finish setting up the io to open the file

* Adds documentation, non-domain decomposed files now only get written by the root PE

* set up the time for the next open and some bug fixes

* Adds code to setups the subaxis

* Fix bugs in the code that get the starting and ending index of the subregion

* fixes bugs in the code that sets up the subaxis object, updates the io for the axis, adds subroutines that dumps the contents of each of the objects

Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants