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

Eliminate repeated diagnostic ID checks and refactor send_diag section #23

Merged
merged 8 commits into from
Apr 8, 2024

Conversation

yichengt900
Copy link
Collaborator

@yichengt900 yichengt900 commented Mar 29, 2024

This PR addresses issue #14. Several code changes are included:

  1. Removed diagnostic ID check lines from the original generic_COBALT_update_from_source and generic_COBALT_update_from_bottom subroutines .
  2. Created a new file cobalt_glbl.F90 and moved phytoplankton, zooplankton, bacteria, and generic_cobalt data types as well as namelist here.
  3. Created a new file cobalt_send_diag.F90 and added a new subroutine cobalt_send_diagnostics to the cobalt_send_diag module to handle send_diag calls separately.
  4. Added a new subroutine cobalt_send_diagnostics_bottom to the cobalt_send_diag module to handle send_diag calls for bottom fluxes.
  5. Reduced the total number of lines in generic_COBALT.F90 from 16169 to 11484 (a 28.91% reduction).
  6. Regarding computational time, the average runtime before and after refactoring in 1d case is 11.50 seconds versus 11.60 seconds, representing an approximate 0.87% increase.
  7. The following namelist capabilities have been removed (mentioned by issue Anomalous namelist usage early in the code to set parameters #20):
k_nh4_small
k_nh4_diazo
k_nh4_large
k_no3_small
k_no3_diazo

I'm still keeping the do_14c option and all the diagnostics related to radiocarbon. We can remove them later if we decide to go in that direction.

This PR should not change baselines (passed CI test) and can produce b2b diagnostic netCDF files.

Eventually we could apply this refactoring to the majority of existing generic_COBALT subroutines (e.g., PR #22 ), which will enhance the clarity and conciseness of the main COBALT code.

@yichengt900 yichengt900 self-assigned this Apr 1, 2024
@yichengt900 yichengt900 marked this pull request as ready for review April 1, 2024 15:02
@yichengt900
Copy link
Collaborator Author

This PR addresses all the comments from reviewers and is up-to-date with the main branch. Please review it again, and feel free to provide any further comments, thanks.

@charliestock
Copy link
Contributor

Thanks for all of your hard work on the Yi-Cheng, all. It looks good from my end.

@yichengt900
Copy link
Collaborator Author

Thanks, @charliestock. I've also merged #39 into this PR. It's ready for merging once one of you approves it.

Copy link
Contributor

@charliestock charliestock left a comment

Choose a reason for hiding this comment

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

This looks good to me Yi-Cheng. Why don't we move forward with merging and discuss further at the doc and dev tomorrow.

@yichengt900 yichengt900 merged commit 1061350 into dev/cefi Apr 8, 2024
1 check passed
@yichengt900 yichengt900 deleted the refactor/send_diag branch April 8, 2024 22:11
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.

4 participants