Workflow for ZPPY- PCMDI for E3SM diagnostics

zhangshixuan1987 commented 2 months ago

Issue resolution

Closes #

Select one: This pull request is...

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

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

Objectives:

Objective 1
Objective 2
...
Objective n

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.

forsyth2 commented 2 months ago

Thanks @zhangshixuan1987 for working on this! Let me know when it's ready for me to review. On my review, I can:

Note code blocks that should be generalized/abstracted with parameters rather than hard-coded (e.g., how to take the code from prototype to production code).
Confirm the checkboxes on the PR template above are fully met.
Confirm the objectives / expected behavior, again noted on the PR template above.

zhangshixuan1987 commented 2 months ago

@forsyth2 : Hi Ryan, based on what you mentioned above, I would like to ask you to review this commit first. This pull request have already included the key part for the workflow:

the control script for job dependency: pcmdi_diags.py
the control template for the main calculation of pcmdi metrics: pcmdi_diags.bash
the parameter files needed for certain process (template/pcmdi_diags/)

I think that it would be better for you to provide some high-level comments on the overall structure and the coding style which should be first fulfilled to meet the requirment by Zppy.

Thank you!

With best wishes,

Shixuan

chengzhuzhang commented 1 month ago

@zhangshixuan1987 and @forsyth2 I will begin reviewing this PR later next week. Shixuan, thank you for your hard work putting together this PR!

zhangshixuan1987 commented 1 month ago

Hi @forsyth2 and @chengzhuzhang : I am not sure if it is properate, but I want to share the sample .cfg files and the run output examples I created for the PCMDI here:

cfg file for v3.LR.hisotical simulation, which is adapted from Jill's cfg file
sample output from the pcmdi, which is applied to the v3.LR.historical simulations during 1985-2014 period.

In addition, I also tested the workflow with AMIP and piControl simulations, which can work reasonably as well. I need to further work on following things:

verify the "model vs model" function
discuss with you and Ryan for a. final www directory structure, and b. the viewer page.

zhangshixuan1987 commented 1 month ago

@chengzhuzhang : Hi Jill, if you check my comments https://github.com/E3SM-Project/zppy/pull/624#issuecomment-2392406822. You will find the e3sm diag cfg file that I shared for you and Ryan to test. please take a look and let me know if you have any questions.

chengzhuzhang commented 3 weeks ago

Hi @forsyth2 and @chengzhuzhang : I am not sure if it is properate, but I want to share the sample .cfg files and the run output examples I created for the PCMDI here:
* [ cfg file for v3.LR.hisotical simulation](https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.szhang/e3sm-pcmdi/post.v3.LR.historical.cfg), which is adapted from Jill's cfg file

* [sample output from the pcmdi](https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.szhang/e3sm-pcmdi/v3.LR.historical_0051/pcmdi_diags/), which is applied to the v3.LR.historical simulations during 1985-2014 period.
In addition, I also tested the workflow with AMIP and piControl simulations, which can work reasonably as well. I need to further work on following things:
1. verify the "model vs model" function

2. discuss with you and Ryan for a. final www directory structure, and b. the viewer page.

When reviewing the results, in the parallel coordinate figure: example. The prw, rldscs, sfcWind show large bias between v3, and CMIP MME, I think the legend might be mixed? Red line should be MME and black for v3?

When going over the results, I agree strongly that it would be much easier to navigate the results with a viewer page! @mingxuanwupnnl

chengzhuzhang commented 3 weeks ago

The *.py files under template/pcmdi_diags are not installed correctly. Example: `jinja2.exceptions.TemplateNotFound: pcmdi_diags/generate_sftlf.py

I think it is due to the constraint in setup:

https://github.com/E3SM-Project/zppy/blob/408e9ade87db8d0d5322c01186564df1392282be/setup.py#L20-L21

By adding "py" to the extensions list, the installation works.

zhangshixuan1987 commented 3 weeks ago

Hi @forsyth2 and @chengzhuzhang : I am not sure if it is properate, but I want to share the sample .cfg files and the run output examples I created for the PCMDI here:
* [ cfg file for v3.LR.hisotical simulation](https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.szhang/e3sm-pcmdi/post.v3.LR.historical.cfg), which is adapted from Jill's cfg file

* [sample output from the pcmdi](https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.szhang/e3sm-pcmdi/v3.LR.historical_0051/pcmdi_diags/), which is applied to the v3.LR.historical simulations during 1985-2014 period.
In addition, I also tested the workflow with AMIP and piControl simulations, which can work reasonably as well. I need to further work on following things:
1. verify the "model vs model" function

2. discuss with you and Ryan for a. final www directory structure, and b. the viewer page.
When reviewing the results, in the parallel coordinate figure: example. The prw, rldscs, sfcWind show large bias between v3, and CMIP MME, I think the legend might be mixed? Red line should be MME and black for v3?

When going over the results, I agree strongly that it would be much easier to navigate the results with a viewer page! @mingxuanwupnnl

Hi @chengzhuzhang : I did not notice this message as you may @ the wrong people. As for your questions here:

Yes, the legend for CMIP MME and v3 in parallel coordinate plots was wrong, the fixed code with improvements in the mean climate metrics plots has been merged to my github branch.
As for the issues for prw, rldscs, sfcWind, this is partly related to the issues in the CMIP6 reference datasets. Jiwoo and I are working together to check the consistency of the units between two datasets, and will update this part.

chengzhuzhang commented 3 weeks ago

I accidentally tagged @mingxuanwupnnl, Sorry!

@zhangshixuan1987 sounds good, I will try test the new PR for now.

E3SM-Project / zppy