Zppy pcmdi new by forsyth2 · Pull Request #718 · E3SM-Project/zppy

forsyth2 · 2025-05-22T17:30:58Z

Summary

This is an alternative to #656 that wants to merge to main rather than zppy_pcmdi (which is #647's branch)

Objectives:

Objective 1
Objective 2
...
Objective n

Issue resolution:

Closes Add task for PMP #141

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

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.

Changes are made in the pcmdi_diags.bash to change the worflow to be more modulized and easier to read

variables were not included in e3sm_to_cmip module

Bug fix to allow the diagnostic workflow work for piControl simulation

forsyth2

@zhangshixuan1987 I made this PR to compare against main (see note at #656 (comment)), since I'm assuming that's what was actually intended.

I took a very high level look, but we're going to have to get this rebased off the latest main before I can really understand what the additions are.

I will try to do what I can in a code clean up commit.

forsyth2 · 2025-05-22T17:32:50Z

@@ -0,0 +1,34 @@
+name: zppy_dev


This file can probably be removed. Is it used anywhere?

forsyth2 · 2025-05-22T17:34:21Z

 [ts]
 area_nm = string(default="area")
 cmip_metadata = string(default="inclusions/e3sm_to_cmip/default_metadata.json")
+cmip_plevdata = string(default="inclusions/e3sm_to_cmip/vrt_remap_plev19.nc")


This code needs to be rebased off the latest main. cmip_metadata isn't handled under ts anymore, but rather under the new e3sm_to_cmip task.

forsyth2 · 2025-05-22T17:50:49Z

+backend = string(default="mpl")
+cfg = string(default="")


Not used; should make sure every parameter is needed

forsyth2 · 2025-05-22T17:55:30Z

@@ -0,0 +1,872 @@
+# Script to plot some global atmosphere and ocean time series


This addition undoes the refactor; I also don't think it's used.

forsyth2 · 2025-05-22T17:55:40Z

Remove change

forsyth2 · 2025-05-22T17:55:51Z

Remove change

forsyth2 · 2025-05-22T17:56:12Z

Remove change

forsyth2 · 2025-05-22T17:56:49Z

+echo ==============================================
+rm -f {{ prefix }}.status
+echo 'OK' > {{ prefix }}.status
+exit 0


We should move some of this into zppy-interfaces. We shouldn't have a 1300-line bash file in zppy; these bash files should be pretty minimal.

forsyth2 · 2025-05-22T17:57:11Z

  mkdir -p ${dest_cmip}
+
  {{ e3sm_to_cmip_environment_commands }}
+


Yes, this needs to be rebased off the refactored code to use the e3sm_to_cmip task.

forsyth2 · 2025-05-22T18:00:31Z

I actually see that e7dda21 is the first commit out of 24 here. That commit is the only one on https://github.com/E3SM-Project/zppy/pull/647/commits, so yes I'm fairly certain the intention was for these commits to go into main instead.

forsyth2 · 2025-08-09T00:04:33Z

Replaced by #719.

zhangshixuan1987 added 24 commits November 26, 2024 12:39

Add PCMDI Diags to zppy

e7dda21

Reformat by pre-commit run

43550e2

Small bug fix in the first line to avoid failure

a9e8708

update the workflow to be modulized

561c82c

Changes are made in the pcmdi_diags.bash to change the worflow to be more modulized and easier to read

move the needed modules to inclusions

b6c638e

clean up the reduandent files for zppy-pcmdi

23b6fa2

remove rltcre and rstcre from the default list of cmip_vars as these

f7fcaaa

variables were not included in e3sm_to_cmip module

adjustment on the workflow

7a52e32

Add revisions on the sythentic plot

2d38e9b

bug fix

67ca00d

reorgnize the including files

205a479

Revise the workflow related to zppy-pcmdi

2a811c8

Revise mpi to ensure the writing process finished

0d3c387

Bug fix

9ff8c18

Bug fix

b2b156d

bug fix

ae24608

Rename fixed directory to be clearer

902b2c5

Change the default set of cmip6 data for synthetic plots

c281986

Refine the workflow

43dafdb

Refine workflow

d30ade2

Add Enso metrics summary plot

bdf224f

Refinement on the code and add viewer page

0c16188

bug fix

ecaa510

Refine the code to move "viewer" into the results directory

ae3e19f

Bug fix to allow the diagnostic workflow work for piControl simulation

forsyth2 mentioned this pull request May 22, 2025

Zppy pcmdi new #656

Closed

15 tasks

forsyth2 commented May 22, 2025

View reviewed changes

forsyth2 mentioned this pull request May 22, 2025

Add PCMDI Diags to zppy #719

Closed

17 tasks

forsyth2 closed this Aug 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zppy pcmdi new#718

Zppy pcmdi new#718
forsyth2 wants to merge 24 commits into
E3SM-Project:mainfrom
zhangshixuan1987:zppy_pcmdi_new

forsyth2 commented May 22, 2025

Uh oh!

forsyth2 left a comment

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 May 22, 2025

Uh oh!

forsyth2 commented May 22, 2025

Uh oh!

forsyth2 commented Aug 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,872 @@
		# Script to plot some global atmosphere and ocean time series

		mkdir -p ${dest_cmip}

		{{ e3sm_to_cmip_environment_commands }}

		backend = string(default="mpl")
		cfg = string(default="")

Conversation

forsyth2 commented May 22, 2025

Summary

Big Change

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

2. Are the implementation details accurate & efficient?

3. Is this well documented?

4. Is this code clean?

Uh oh!

forsyth2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

forsyth2 commented May 22, 2025

Uh oh!

forsyth2 commented Aug 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants