Skip to content

Add precip_pdf and mp_partition from e3sm diags#787

Merged
chengzhuzhang merged 6 commits intomainfrom
feature/add_precip-pdf_mp-partition_from_e3sm-diags
Mar 24, 2026
Merged

Add precip_pdf and mp_partition from e3sm diags#787
chengzhuzhang merged 6 commits intomainfrom
feature/add_precip-pdf_mp-partition_from_e3sm-diags

Conversation

@chengzhuzhang
Copy link
Copy Markdown
Collaborator

@chengzhuzhang chengzhuzhang commented Mar 3, 2026

Summary

Implemented PR scope to add two new e3sm_diags sets in zppy: first mp_partition (from e3sm_diags PR #1028), then precip_pdf (from PR #1023).
Changes include wiring both sets into zppy/templates/e3sm_diags.bash (imports + parameter objects + monthly/daily TS paths + model-vs-model reference wiring), adding dependency/path inference and required-parameter logic in zppy/e3sm_diags.py (mp_partition uses monthly TS deps, precip_pdf uses daily TS deps), updating available-set docs comment in zppy/defaults/default.ini, and extending unit coverage in tests/test_zppy_e3sm_diags.py for mvm checks, inferred reference paths, and TS dependency generation for both sets.

The results can be viewed here: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/zppy_example/v3.2.0/v3.LR.historical_0051/

Issue resolution:

Select one: This pull request is...

  • a bug fix: increment the patch version
  • a small improvement: increment the minor version
  • a new feature: increment the minor version
  • an incompatible (non-backwards compatible) API change: increment the major version

Please fill out either the "Small Change" or "Big Change" section (the latter includes the numbered subsections), and delete the other.

Small Change

  • To merge, I will use "Squash and merge". That is, this change should be a single commit.
  • Logic: I have visually inspected the entire pull request myself.
  • Pre-commit checks: All the pre-commits checks have passed.

Big Change

  • To merge, I will use "Create a merge commit". That is, this change is large enough to require multiple units of work (i.e., it should be multiple commits).

1. Does this do what we want it to do?

Required:

  • Product Management: I have confirmed with the stakeholders that the objectives above are correct and complete.
  • Testing: I have added or modified at least one "min-case" configuration file to test this change. Every objective above is represented in at least one cfg.
  • Testing: I have considered likely and/or severe edge cases and have included them in testing.

If applicable:

  • Testing: this pull request introduces an important feature or bug fix that we must test often. I have updated the weekly-test configuration files, not just a "min-case" one.
  • Testing: this pull request adds at least one new possible parameter to the cfg. I have tested using this parameter with and without any other parameter that may interact with it.

2. Are the implementation details accurate & efficient?

Required:

  • Logic: I have visually inspected the entire pull request myself.
  • Logic: I have left GitHub comments highlighting important pieces of code logic. I have had these code blocks reviewed by at least one other team member.

If applicable:

  • Dependencies: This pull request introduces a new dependency. I have discussed this requirement with at least one other team member. The dependency is noted in zppy/conda, not just an import statement.

3. Is this well documented?

Required:

  • Documentation: by looking at the docs, a new user could easily understand the functionality introduced by this pull request.

4. Is this code clean?

Required:

  • Readability: The code is as simple as possible and well-commented, such that a new team member could understand what's happening.
  • Pre-commit checks: All the pre-commits checks have passed.

If applicable:

  • Software architecture: I have discussed relevant trade-offs in design decisions with at least one other team member. It is unlikely that this pull request will increase tech debt.

@chengzhuzhang chengzhuzhang changed the title Feature/add precip pdf mp partition from e3sm diags Add precip_pdf and mp_partition from e3sm diags Mar 3, 2026
@chengzhuzhang chengzhuzhang marked this pull request as ready for review March 18, 2026 22:47
years = "1985:2014:30",
# NOTE: If you want to use the latest development environment of e3sm_diags,
# you can do something like the following:
environment_commands = "source /home/ac.zhang40/y/etc/profile.d/conda.sh; conda activate e3sm_diags_dev_py313"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Need to comment out after e3sm_unified released

@chengzhuzhang chengzhuzhang requested a review from forsyth2 March 18, 2026 22:49
@chengzhuzhang
Copy link
Copy Markdown
Collaborator Author

@forsyth2 i believe this is ready to review. Let me know if code change makes sense, and the results can be viewed here: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/zppy_example/v3.2.0/v3.LR.historical_0051/. Thank you!

Copy link
Copy Markdown
Collaborator

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

Thanks @chengzhuzhang. By visual inspection, the code changes look reasonable. I'll add a commit to include the new sets in the integration test cfg. Currently waiting for nodes on Chrysalis to run.

Copy link
Copy Markdown
Collaborator

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

@chengzhuzhang I've confirmed I can produce results using an updated test cfg:

Once this PR is merged, as part of main-branch testing I'll actually update expected results.

I've added a new commit to this PR that includes the relevant test cfg changes. I think this should be good to merge now. Thanks!

@chengzhuzhang
Copy link
Copy Markdown
Collaborator Author

@forsyth2 thank you for reviewing and testing!

@chengzhuzhang chengzhuzhang merged commit 0f8964f into main Mar 24, 2026
7 checks passed
@chengzhuzhang chengzhuzhang deleted the feature/add_precip-pdf_mp-partition_from_e3sm-diags branch March 24, 2026 01:57
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.

Add e3sm_diags Precipitation PDF plots to integration tests

2 participants