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

Add TUV-x height grid updates #134

Merged
merged 29 commits into from
Oct 23, 2024
Merged

Add TUV-x height grid updates #134

merged 29 commits into from
Oct 23, 2024

Conversation

boulderdaze
Copy link
Collaborator

@boulderdaze boulderdaze commented Oct 15, 2024

Updates TUV-x height grid with CAM-SIMA state data before calculating rate constants.

Uses geopontential height values to calculate actual heights above sea level, as currently done in the CAMChem gas-phase chemistry module. If there is a better way to get these height values, we can update this PR.

Closes #94


Notes (Jiwon):

  • Replaced TUV-xedges with interface
  • Added a valgrind suppression option to suppress memory leak issues in certain functions of TUV-x.


type(grid_t), intent(inout) :: height_grid
real(kind_phys), intent(in) :: host_midpoints(:) ! km
real(kind_phys), intent(in) :: host_edges(:) ! km
Copy link
Collaborator Author

@boulderdaze boulderdaze Oct 15, 2024

Choose a reason for hiding this comment

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

Is the term edge equivalent to interface? If so, can we use interface across the code base to enhance the consistency and readability?

Suggested change
real(kind_phys), intent(in) :: host_edges(:) ! km
real(kind_phys), intent(in) :: host_interface(:) ! km

@boulderdaze boulderdaze self-assigned this Oct 16, 2024
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Looks good, but did have a few questions and some suggestions. Of course please let me know if you have any concerns with any of my comments. Thanks!

schemes/musica/micm/musica_ccpp_micm_util.F90 Outdated Show resolved Hide resolved
schemes/musica/musica_ccpp.F90 Outdated Show resolved Hide resolved
schemes/musica/musica_ccpp.meta Outdated Show resolved Hide resolved
schemes/musica/tuvx/musica_ccpp_tuvx.F90 Show resolved Hide resolved
schemes/musica/tuvx/musica_ccpp_tuvx.F90 Outdated Show resolved Hide resolved
schemes/musica/musica_ccpp.meta Outdated Show resolved Hide resolved
test/docker/Dockerfile.musica Show resolved Hide resolved
test/docker/Dockerfile.musica Outdated Show resolved Hide resolved
test/docker/Dockerfile.musica.no_install Outdated Show resolved Hide resolved
test/musica/micm/test_micm_util.F90 Outdated Show resolved Hide resolved
@@ -128,7 +129,7 @@ subroutine tuvx_run( temperature, dry_air_density, &
call calculate_heights( geopotential_height_wrt_surface_at_midpoint(i_col,:), &
geopotential_height_wrt_surface_at_interface(i_col,:), &
surface_geopotential(i_col), &
reciprocal_of_gravitational_acceleration, &
standard_gravitational_acceleration, &
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would probably recommend calculating reciprocal_of_gravitational_acceleration outside of the i_col loop and then just passing it to calculate_heights like you were doing previously (this could potentially reduce the total number of divisions that have to be done).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, thank you for the comment!

test/docker/Dockerfile.musica Show resolved Hide resolved
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Everything looks good to me now! I did have one final performance-related change request, but it certainly isn't required (and doesn't need a re-review). Thanks again!

Copy link
Collaborator

@K20shores K20shores left a comment

Choose a reason for hiding this comment

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

looks good to me!

Copy link
Collaborator

@mattldawson mattldawson left a comment

Choose a reason for hiding this comment

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

looks great!

@boulderdaze boulderdaze merged commit fbcf356 into development Oct 23, 2024
2 checks passed
@boulderdaze boulderdaze deleted the 94-tuvx-height-grid branch October 23, 2024 16:50
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