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 the bug: TUV photolysis is inactive if simulation starts on 1st Jan #2171

Open
wants to merge 2 commits into
base: release-v4.7.0
Choose a base branch
from

Conversation

SeregaOsipov
Copy link
Contributor

Fix the bug: photolysis is inactive if simulation starts on 1st Jan

TYPE: bug fix

KEYWORDS: TUV, distance to the sun, 1 January, Julian day

SOURCE: Sergey Osipov (KAUST)

DESCRIPTION OF CHANGES:
Problem:
TUV and FTUV fail to initialize the distance to the Sun properly if the simulation starts on 1 Jan. The multiplication factor remains uninitialized at 0, which zeros out photolysis rate calculations (see line 745 @ module_phot_tuv.F. On the next day, the distance is recalculated normally and photolysis rates are calculated properly.

Solution:
Given that the calculation of sun distance is trivial, the if check was disabled

ISSUE:

LIST OF MODIFIED FILES:
chem/module_phot_tuv.F
chem/module_ftuv_driver.F

TESTS CONDUCTED:

  1. Do mods fix problem? How can that be demonstrated, and was that test conducted?
  2. Are the Jenkins tests all passing?

RELEASE NOTE: Include a stand-alone message suitable for the inclusion in the minor and annual releases. A publication citation is appropriate.

TYPE: bug fix

KEYWORDS: TUV, distance to sun, 1 January, julian day

SOURCE: Sergey Osipov (KAUST)

DESCRIPTION OF CHANGES:
Problem:
TUV and FTUV fail to initialize the distance to Sun properly if the simulation starts on 1 Jan. The multiplication factor remains uninitialized at 0, which zeros out photolysis rate calculations (see line 745 @ module_phot_tuv.F. On a next day the distance is recalculated normally.

Solution:
Given that calculation of sun distance is trivial, if check was disabled

ISSUE:

LIST OF MODIFIED FILES:
chem/module_phot_tuv.F
chem/module_ftuv_driver.F

TESTS CONDUCTED:
1. Do mods fix problem? How can that be demonstrated, and was that test conducted?
2. Are the Jenkins tests all passing?

RELEASE NOTE: Include a stand-alone message suitable for the inclusion in the minor and annual releases. A publication citation is appropriate.
TYPE: bug fix

KEYWORDS: TUV, distance to sun, 1 January, julian day

SOURCE: Sergey Osipov (KAUST)

DESCRIPTION OF CHANGES:
Problem:
TUV and FTUV fail to initialize the distance to Sun properly if the simulation starts on 1 Jan. The multiplication factor remains uninitialized at 0, which zeros out photolysis rate calculations (see line 745 @ module_phot_tuv.F. On a next day the distance is recalculated normally.

Solution:
Given that calculation of sun distance is trivial, if check was disabled

ISSUE:

LIST OF MODIFIED FILES:
chem/module_phot_tuv.F
chem/module_ftuv_driver.F

TESTS CONDUCTED:
1. Do mods fix problem? How can that be demonstrated, and was that test conducted?
2. Are the Jenkins tests all passing?

RELEASE NOTE: Include a stand-alone message suitable for the inclusion in the minor and annual releases. A publication citation is appropriate.
@SeregaOsipov SeregaOsipov requested a review from a team as a code owner February 26, 2025 06:31
@weiwangncar
Copy link
Collaborator

The regression test results:
Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 23 24
Number of Builds : 60 57
Number of Simulations : 158 150 0
Number of Comparisons : 95 86 0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

@weiwangncar
Copy link
Collaborator

@jordanschnell Just wondering if you could help review this PR? Thanks!

@jordanschnell
Copy link
Contributor

Hi Sergey, thanks for this fix. Instead of commenting out the check such that it is called each time, what about including a check for julday = 0?

@SeregaOsipov
Copy link
Contributor Author

Hi Jordan, good to hear from you.

Julday is zero if the simulation start date is 1st Jan. At least, this is what I saw in the DDT debugger on our supercomputer.
The value is set in line 346 in chem_driver.F: ijulian=ifix(grid%julian) and then passed to the photolysis_driver.F

An alternative is to initialize the curjulday with -1 (or -999) instead of 0, which is used as a cache for the last calculation.

Overall, the sundist calculation is very trivial and only involves trigonometric functions, meaning that the computatinal overhead is negligeble. IMHO, this is a good trade-off to avoid the hard-to-predict bugs like the 1st January case.

Best, Sergey

jordanschnell
jordanschnell previously approved these changes Feb 26, 2025
Copy link
Contributor

@jordanschnell jordanschnell left a comment

Choose a reason for hiding this comment

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

OK with me. Approved.

@weiwangncar weiwangncar changed the base branch from develop to release-v4.7.0 February 27, 2025 17:44
@weiwangncar weiwangncar dismissed jordanschnell’s stale review February 27, 2025 17:44

The base branch was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants