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

Improve diagnostics #403

Closed
wants to merge 4 commits into from

Conversation

mnlevy1981
Copy link
Collaborator

This PR addresses two small issues with diagnostics:

  1. fixes photoC_NO3_TOT_zint_100m diagnostic not computed #401 (photoC_NO3_TOT_zint_100m was being defined but never computed)
  2. resolves Output FEISTY forcing directly #400 (pocToFloor is now output in the medium stream by default)

For the first, I opted to compute photoC_NO3_TOT_zint_100m rather than remove it from the diagnostic list.

There are two small issues remaining to address:

  • I still need to update the baseline file to avoid an error when comparing results from tests/regression_tests/call_compute_subroutines/
  • There is a CI test failure in building the documentation that I think stems from an incompatibility between the version of sphinx we are using for the docs and a major version update of jinja2.

I'll remove draft status from this PR after fixing those two minor issues.

This diagnostic was added to the diagnostics list, but never actually computed.
I added an additional near_surface_integral argument to where we compute the
per-autotroph photoC_NO3_zint (full depth integral), and the sum of those
near_surface_integral values is the _100m equivalent.
frequency was changed from 'none' to 'medium' because this value is useful for
forcing FEISTY and it would be nice to just have it in typical CESM output
I was seeing an error that was introduced with jinja2 v3.0 in the CI, so I
require <3 in the pip requirements file. In a sandbox, I was then seeing an
error from MarkupSafe and it turns out that was introduced in v2.1 so I require
<2.1 in the pip file.

I also updated the github action to (hopefully) run the Fortran test suite even
if building the documentation fails
a3dde53 changed the value of photoC_NO3_TOT_zint_100m from 0 to the actual
integral that should be computed, and we need those correct values in the
baseline for testing purposes.
@mnlevy1981 mnlevy1981 marked this pull request as ready for review May 10, 2022 20:24
@mnlevy1981
Copy link
Collaborator Author

After talking with @klindsay28, we thought the best plan would be to

  1. Bring in the CI changes in a separate PR (Fix CI #404)
  2. Discuss the two other changes in this PR on Tuesday

I'm going to close this PR, and will open a new one (or two) next week, depending on how the conversation goes.

@mnlevy1981 mnlevy1981 closed this May 12, 2022
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.

photoC_NO3_TOT_zint_100m diagnostic not computed Output FEISTY forcing directly
1 participant