Skip to content

Modern diag manager: add subzaxis#1148

Merged
thomas-robinson merged 23 commits into
NOAA-GFDL:dmUpdatefrom
uramirez8707:add_subzaxis
Mar 10, 2023
Merged

Modern diag manager: add subzaxis#1148
thomas-robinson merged 23 commits into
NOAA-GFDL:dmUpdatefrom
uramirez8707:add_subzaxis

Conversation

@uramirez8707
Copy link
Copy Markdown
Contributor

Description

  • Remove subRegion from the subaxis type because I was not really using it
  • Adds code to fill in the z subaxis
  • Adds a test for it

Fixes # (issue)

How Has This Been Tested?
CI including added test

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

@uramirez8707 uramirez8707 changed the title Add subzaxis Modern diag manager: add subzaxis Mar 8, 2023
Copy link
Copy Markdown
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.

I'm a little concerned about the mixed-mode support here and the real casting.

Comment thread diag_manager/fms_diag_axis_object.F90
Comment thread diag_manager/fms_diag_axis_object.F90
Comment thread diag_manager/fms_diag_axis_object.F90
Comment thread diag_manager/fms_diag_axis_object.F90
Comment on lines +1243 to +1249
type is (real(kind=r4_kind))
!TODO need to include the conversion to "real" because nearest_index doesn't take r4s and r8s
subaxis_indices(1) = nearest_index(real(zbounds(1)), real(zaxis_data))
subaxis_indices(2) = nearest_index(real(zbounds(2)), real(zaxis_data))
type is (real(kind=r8_kind))
subaxis_indices(1) = nearest_index(real(zbounds(1)), real(zaxis_data))
subaxis_indices(2) = nearest_index(real(zbounds(2)), real(zaxis_data))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there may be an issue here with the types and just using real()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Once axis_utils2 is updated for mixed mode, the conversion to real can be done correctly:

          type is (real(kind=r4_kind))
            !TODO need to include the conversion to "real" because nearest_index doesn't take r4s and r8s
            subaxis_indices(1) = nearest_index(zbounds(1), zaxis_data)
            subaxis_indices(2) = nearest_index(zbounds(2), zaxis_data)
          type is (real(kind=r8_kind))
            subaxis_indices(1) = nearest_index(real(zbounds(1), kind=r8_kind), zaxis_data)
            subaxis_indices(2) = nearest_index(real(zbounds(2), kind=r8_kind), zaxis_data)

Right now nearest_index only takes in "reals", so this won't compile ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, I think I understand now. We need to keep track of this.

Comment thread diag_manager/fms_diag_field_object.F90 Outdated
axis_ptr => diag_axis(this%axis_ids(i))
dimnames(i) = axis_ptr%axis%get_axis_name(is_regional)
enddo
!< Duplicated do loops for #performance
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the # going to mess with any preprocessing or text editor highlighting? Is something like VScode actually going to read this as a hash? I don't actually know.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok will remove

@thomas-robinson thomas-robinson merged commit 5ce4790 into NOAA-GFDL:dmUpdate Mar 10, 2023
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
* write the time data based on how FMS is compiled

* Implement capability to specifiy which time in a time average period to use when setting filename

* Fix race conditions + add send_data tests

*  add the sub z axis

* Add edges_names when defining the diurnal axis

* Fix string length and adds some more documentation updates

* simplify the register_field call
rem1776 pushed a commit to rem1776/FMS that referenced this pull request May 1, 2024
* write the time data based on how FMS is compiled

* Implement capability to specifiy which time in a time average period to use when setting filename

* Fix race conditions + add send_data tests

*  add the sub z axis

* Add edges_names when defining the diurnal axis

* Fix string length and adds some more documentation updates

* simplify the register_field call
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