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

modern_diag_manager: fms_diag_do_reduction #1321

Merged
merged 8 commits into from
Aug 11, 2023

Conversation

uramirez8707
Copy link
Contributor

Description
This contains part of #1295

  • adds getters functions to get the subaxis starting and ending indices
  • adds the fms_diag_reduction_methods_module
  • adds checks to make sure the input arguments are passed in correctly to send_data
  • adds the call to fms_diag_do_reduction

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

diag_manager/diag_manager.F90 Outdated Show resolved Hide resolved
diag_manager/fms_diag_axis_object.F90 Outdated Show resolved Hide resolved
diag_manager/fms_diag_axis_object.F90 Outdated Show resolved Hide resolved
diag_manager/fms_diag_object.F90 Outdated Show resolved Hide resolved
diag_manager/fms_diag_object.F90 Show resolved Hide resolved
diag_manager/fms_diag_reduction_methods.F90 Show resolved Hide resolved
Comment on lines +61 to +62
LOGICAL, DIMENSION(:,:,:,:), pointer, INTENT(in) :: mask !< The location of the mask
CLASS(*), DIMENSION(:,:,:,:), pointer, INTENT(in) :: rmask !< The masking values
Copy link
Member

Choose a reason for hiding this comment

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

do these have to be pointer? Is it just because their input arguments are pointers? Is that a requirement in Fortran? Since Fortran is pass by reference, I don't think these need to be pointers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is required because I am doing if(associated(mask))

diag_manager/fms_diag_reduction_methods.F90 Outdated Show resolved Hide resolved

real(kind=r8_kind) :: out_weight

out_weight = 1.0_r8_kind
Copy link
Member

Choose a reason for hiding this comment

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

Did you run this by @mlee03 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.0_r8_kind is preferred over real(1.0 kind=r8_kind)

type is (real(kind=r8_kind))
out_weight = real(weight, kind = r8_kind)
type is (real(kind=r4_kind))
out_Weight = real(weight, kind = r4_kind)
Copy link
Member

Choose a reason for hiding this comment

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

You declared out_Weight as r8, but here you are making it r4? Maybe you need out_Weight to be class(*) and allocate it to whatever weight is? Then out_weight = 1.0_r8_kind would be in the class default branch of the select type. Otherwise you should have kind=r8_kind on this line I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

out_weight is going to be saved as a r8_kind and converted to r4_kind if needed when doing the mad.

That should be kind=r8_kind no kind=r4_kind

@uramirez8707
Copy link
Contributor Author

@thomas-robinson I think everything was addressed in 91a249a

field_modern(1:ie,1:je,1:ke,1:1) => field
field_remap(1:ie,1:je,1:ke,1:1) => field
if (present(mask)) mask_remap(1:ie,1:je,1:ke,1:1) => mask
if (present(rmask)) rmask_remap(1:ie,1:je,1:ke,1:1) => rmask
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 rmask declared with contiguous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure why the compiler didn't complain, but i declared it ascontiguous

@thomas-robinson
Copy link
Member

@uramirez8707 There is a merge conflict

@uramirez8707
Copy link
Contributor Author

@thomas-robinson the "merge conflict" was resolved

@thomas-robinson thomas-robinson merged commit f28b99b into NOAA-GFDL:dmUpdate Aug 11, 2023
17 of 18 checks passed
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
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