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

fix bug when calling unallocated diagnostics #50

Merged
merged 3 commits into from
Jun 28, 2024

Conversation

JosephMouallem
Copy link
Contributor

Description

The subroutine compute_column_integrated_moles_of_dry_air_and_co2 is called by default to compute:
Diag%column_moles_dry_air_per_square_meter and Diag%column_moles_co2_per_square_meter in GFS_layer/GFS_radiation_driver.F90:

These quantities are not allocated in GFS_layer/GFS_typedefs.F90 if ldiag3d is false in gfs_physics_nml.
Thus, this causes a code crash.
This PR will add an if statement before calling compute_column_integrated_moles_of_dry_air_and_co2

Tested within shield in debug mode with the current public release branches.

Checklist:

Please check all whether they apply or not

  • 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

Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

Oops sorry @JosephMouallem this was my bad...how do you feel about flipping this (i.e. always allocating these fields in GFS_typedefs.F90) such that one does not need to set ldiag3d = .true. to use the global_mean_co2 diagnostic?

I changed my mind midway through implementing it in #45, and apparently neglected to test it with ldiag3d = .false., leaving it in this semi-broken state.

@lharris4
Copy link
Contributor

@spencerkclark Since this is a 0D and not a 3D diagnostic, the best way is if we don't need to set ldiag3d to get this diagnostic.

@spencerkclark
Copy link
Member

Thanks @lharris4—I agree. Sorry for not making my reasoning clearer.

@JosephMouallem
Copy link
Contributor Author

Thanks @spencerkclark @lharris4 for sharing you thoughts. I moved these fields out of ldiag3d.
@spencerkclark no worries. Please let know if you would like to make any other changes.

Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

Thanks @JosephMouallem—this looks good to me now!

Copy link
Contributor

@linjiongzhou linjiongzhou left a comment

Choose a reason for hiding this comment

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

Hi Joseph and Spencer,

Thanks for your effort on this. It looks good to me.

Linjiong

@spencerkclark
Copy link
Member

A gentle ping on this, which I think should be a quick merge.

@lharris4 lharris4 merged commit fe50c7f into NOAA-GFDL:main Jun 28, 2024
2 checks passed
@lharris4
Copy link
Contributor

@spencerkclark sorry for the delay. I have completed the merge.

@spencerkclark
Copy link
Member

All good, just wanted to make sure it didn't get lost. Thanks!

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.

4 participants