Solar zenith angle and Earth-Sun distance #171

mattldawson · 2024-11-27T21:59:14Z

Adds the calculation of solar zenith angle and Earth-Sun distance needed as inputs to TUV-x.

closes #163
closes #164

Both calculations required functions that are currently in shared code in CAM here and here that I put in a modified form in the to_be_ccppized/ folder. @nusbaume @peverwhee - if there is a better way to handle these functions, let me know and I can update the PR.

Requires an update to CAM-SIMA that is in review to make certain variables available through CCPP (ESCOMP/CAM-SIMA#325)

jimmielin

Thanks @mattldawson! I have a couple of minor suggestions but nothing that holds up the PR. Just a reminder that the Earth eccentricity, etc. parameter need a SIMA update (ESCOMP/CAM-SIMA#325) to be merged before they're available for use by code in this PR.

jimmielin · 2024-11-27T22:05:27Z

schemes/musica/musica_ccpp_util.F90

-  public :: has_error_occurred
+  public :: has_error_occurred, calculate_solar_zenith_angle_and_earth_sun_distance
+
+  real(kind_phys), parameter, public :: PI = 3.14159265358979323846_kind_phys


I would suggest avoid hardcoding this in a separate location and use the CCPP standard name pi_constant if possible, or if not, maybe include from shr_const_pi (these are the same numerically). I realize this is used in another module and maybe the CCPP variable can be brought in from there.

I guess my preference would be to use the shr_const_pi instead of adding yet another function argument to a list that is getting pretty long, and then having to pass constants down through all the internal functions, but I'll go with the approach the group prefers.

jimmielin · 2024-11-27T22:06:11Z

schemes/musica/tuvx/musica_ccpp_tuvx.F90

@@ -129,6 +132,7 @@ subroutine tuvx_init(vertical_layer_dimension, vertical_interface_dimension, &
    use musica_tuvx, only: grid_map_t, profile_map_t, radiator_map_t
    use musica_util, only: error_t, configuration_t
    use musica_ccpp_namelist, only: filename_of_tuvx_micm_mapping_configuration
+    use musica_ccpp_util, only: PI


Maybe use CCPP variable pi_constant as an argument, if possible?

jimmielin · 2024-11-27T22:06:44Z

schemes/musica/tuvx/musica_ccpp_tuvx.F90

                      cloud_area_fraction, constituents,             &
                      air_pressure_thickness, rate_parameters,       &
                      errmsg, errcode)
    use musica_util,                               only: error_t
    use musica_ccpp_tuvx_height_grid,              only: set_height_grid_values, calculate_heights
    use musica_ccpp_tuvx_temperature,              only: set_temperature_values
-    use musica_ccpp_util,                          only: has_error_occurred
+    use musica_ccpp_util,                          only: has_error_occurred, PI


Similarly, would it be possible to use the CCPP pi_constant here as well?

jimmielin · 2024-11-27T22:16:11Z

to_be_ccppized/shr_orb_mod.F90

@@ -0,0 +1,141 @@
+! Copyright (C) 2024 National Science Foundation-National Center for Atmospheric Research
+! SPDX-License-Identifier: Apache-2.0
+module shr_orb_mod


I suppose this is fine since it's a temporary solution but I wonder if the copy of shr_orb_mod in CAM-SIMA's share submodule can be used directly here, perhaps through a dependency (dependencies = ../../../../../share/shr_orb_mod.F90 in the ccpp metadata. Just wanted to open it up for discussion in case it's an easy fix. Please don't hold up the PR if not, I think this is best computed in the host model and passed as a CCPP variable eventually (e.g., cosine_of_solar_zenith_angle)

I would be a little hesitant to introduce this type of dependency on source code and a very specific folder structure of a host application. But, I agree with the long-term plan to pass this as a CCPP variable.

mattldawson · 2024-11-27T22:29:29Z

Thanks @mattldawson! I have a couple of minor suggestions but nothing that holds up the PR. Just a reminder that the Earth eccentricity, etc. parameter need a SIMA update (ESCOMP/CAM-SIMA#325) to be merged before they're available for use by code in this PR.

Good point. I added a note to the PR description. thanks!

boulderdaze

Looks good! I just have a couple of suggestions and questions.

boulderdaze · 2024-12-02T17:44:55Z

schemes/musica/musica_ccpp.F90

+    real(kind_phys),         intent(in)    :: longitude(:)                                      ! radians (column)
+
+    real(kind_phys),         intent(in)    :: earth_eccentricity                                ! Earth's eccentricity factor (unitless) (typically 0 to 0.1)


Maybe remove the blank line between lines 108 and 110?

boulderdaze · 2024-12-02T17:57:45Z

schemes/musica/musica_ccpp.F90

@@ -121,6 +140,8 @@ subroutine musica_ccpp_run(time_step, temperature, pressure, dry_air_density, co
                  photolysis_wavelength_grid_interfaces,         &
                  extraterrestrial_flux,                         &
                  standard_gravitational_acceleration,           &
+                  solar_zenith_angle,                            &
+                  earth_sun_distance,                            &


I think it would be nice if the sequence of function parameters in the muscia_ccpp_run matches that of tuvx_run, as it’s more intuitive. In muscia_ccpp_run, cloud area fraction argument appear before solar zenith angle-related arguments, but in tuvx_run, it comes after. Does it make sense to align the sequences?

boulderdaze · 2024-12-02T18:08:08Z

schemes/musica/musica_ccpp_util.F90

+    call shr_orb_decl(calendar_day, earth_eccentricity, earth_obliquity, &
+                      perihelion_longitude, moving_vernal_equinox_longitude, &
+                      delta, earth_sun_distance)


This function is imported, but if we or CAM-SIMA are going to rewrite it, is there a chance we could rename the function?

boulderdaze · 2024-12-02T18:22:55Z

schemes/musica/tuvx/musica_ccpp_tuvx.F90

                                   surface_temperature(i_col), errmsg, errcode )
-      if (errcode /= 0) return
+        if (errcode /= 0) return

      call set_cloud_optics_values( cloud_optics, cloud_area_fraction(i_col,:), &


Could you add space in the lines starting 498 - 503 to align with the else conditional code?

boulderdaze · 2024-12-02T18:37:28Z

test/musica/test_musica_api.F90

+                          standard_gravitational_acceleration, cloud_area_fraction, air_pressure_thickness, latitude, longitude, earth_eccentricity, &
+                          earth_obliquity, perihelion_longitude, moving_vernal_equinox_longitude,       &
+                          calendar_day, errmsg, errcode )


Would it be possible to split this list of arguments (line 339) into two lines?

boulderdaze · 2024-12-02T18:40:08Z

test/musica/test_musica_api.F90

+    ! Set conditions for one daytime and one nighttime column
+    ! Greenwich, UK and Wellington, NZ
+    latitude = (/ 51.5_kind_phys, -41.3_kind_phys /)
+    longitude = (/ 0.0_kind_phys, 174.8_kind_phys /)
+    earth_eccentricity = 0.0167_kind_phys
+    earth_obliquity = 23.5_kind_phys * DEGREE_TO_RADIAN
+    perihelion_longitude = 102.9_kind_phys * DEGREE_TO_RADIAN
+    moving_vernal_equinox_longitude = 182.7_kind_phys * DEGREE_TO_RADIAN
+    calendar_day = 183.5_kind_phys ! noon GMT Jul 1
+


These are duplicates (lines 478–486). Could you remove them?

boulderdaze · 2024-12-02T18:41:29Z

test/musica/test_musica_api.F90

+                          standard_gravitational_acceleration, cloud_area_fraction, air_pressure_thickness, latitude, longitude, earth_eccentricity, &
+                          earth_obliquity, perihelion_longitude, moving_vernal_equinox_longitude,       &


The same request for splitting the arguments into a couple of lines.

mattldawson added 5 commits November 26, 2024 15:02

draft sza and earth-sun distance code

8063a01

finish sza and earth-sun distance changes

073eb1d

finish tests

da7b2c6

Merge branch 'development' into develop-163-solar-zenith-angle

372c4ca

fix build script

19a0cc2

mattldawson requested review from jimmielin, peverwhee, nusbaume and boulderdaze November 27, 2024 21:59

formatting and test conditions

6538d52

jimmielin approved these changes Nov 27, 2024

View reviewed changes

boulderdaze requested changes Dec 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solar zenith angle and Earth-Sun distance #171

Solar zenith angle and Earth-Sun distance #171

mattldawson commented Nov 27, 2024 •

edited

Loading

jimmielin left a comment

jimmielin Nov 27, 2024

mattldawson Nov 27, 2024

jimmielin Nov 27, 2024

jimmielin Nov 27, 2024

jimmielin Nov 27, 2024

mattldawson Dec 2, 2024

mattldawson commented Nov 27, 2024

boulderdaze left a comment

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

boulderdaze Dec 2, 2024

		real(kind_phys), intent(in) :: longitude(:) ! radians (column)

		real(kind_phys), intent(in) :: earth_eccentricity ! Earth's eccentricity factor (unitless) (typically 0 to 0.1)

		standard_gravitational_acceleration, cloud_area_fraction, air_pressure_thickness, latitude, longitude, earth_eccentricity, &
		earth_obliquity, perihelion_longitude, moving_vernal_equinox_longitude, &

Solar zenith angle and Earth-Sun distance #171

Are you sure you want to change the base?

Solar zenith angle and Earth-Sun distance #171

Conversation

mattldawson commented Nov 27, 2024 • edited Loading

jimmielin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattldawson commented Nov 27, 2024

boulderdaze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattldawson commented Nov 27, 2024 •

edited

Loading