Split global_time_series task #722
Conversation
| [[__many__]] | ||
| climo_years = string_list(default=None) | ||
| color = string(default=None) | ||
| experiment_name = string(default=None) | ||
| figstr = string(default=None) | ||
| input_subdir = string(default=None) | ||
| make_viewer = boolean(default=None) | ||
| moc_file = string(default=None) | ||
| ncols = integer(default=None) | ||
| nrows = integer(default=None) | ||
| plots_original = string(default=None) | ||
| plots_atm = string(default=None) | ||
| plots_ice = string(default=None) | ||
| plots_lnd = string(default=None) | ||
| plots_ocn = string(default=None) | ||
| regions = string(default=None) | ||
| ts_years = string_list(default=None) | ||
|
|
There was a problem hiding this comment.
I'm a little stuck here:
Trying to run multiple subtasks
I'm trying to add multiple subtasks to the global_time_series task because it will make testing (both of E3SM-Project/zppy-interfaces#26 and in general) much more efficient. Currently, since only one global_time_series task can be set per cfg, each cfg testing a different case on the same input just ends up running ts and mpas_analysis unnecessarily.
__many__ in default.ini, no subsections in cfg
So, I tried adding the __many__ section here, but leaving the cfg without a subsection ended up causing mpas_analysis to fail! Specifically:
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/zppy-gts-split-20250603-try2/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# global_time_series_1985-1995.status:WAITING 755743
# mpas_analysis_ts_1985-1995_climo_1990-1995.status:ERROR (1)
emacs mpas_analysis_ts_1985-1995_climo_1990-1995.o755744 # The second mpas_analysis job's outputgives:
Traceback (most recent call last):
File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.1_chrysalis/bin/mpas_analysis", line 10, in <module>
sys.exit(main())
File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.1_chrysalis/lib/python3.10/site-packages/mpas_analysis/__main__.py", line 1194, in main
run_analysis(config, analyses)
File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.1_chrysalis/lib/python3.10/site-packages/mpas_analysis/__main__.py", line 736, in run_analysis
assert(runningProcessCount > 0)
AssertionError
I tried running again and the same error occurred.
__many__ in default.ini, with subsection in cfg
Then, I added a subsection (which is [gts_subtask] in tests/integration/template_weekly_comprehensive_v3.cfg in this PR), and everything went back to working:
cd /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/zppy-gts-split-20250603-try3/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# No issuesThe output PDF looks fine.
So, that would be fine except it breaks backwards compatibility -- we'd have to increment zppy's major version and communicate a breaking change yet again. (Side note: It may make sense to implement #681 (comment) as part of or concurrent with this PR).
Analysis
Looking at the __many__'s documentation isn't helping me too much.
In theory, if no subsection is specified, zppy should just use the overarching task, based on this code block in https://github.com/E3SM-Project/zppy/blob/main/zppy/utils.py:
if len(sub_section_names) == 0:
# No sub-section, single task
# Merge current section with default. Work with copies to avoid contamination
task = config["default"].copy()
task.update(config[section_name].copy())
# Set 'subsection' in dictionary to None
task["subsection"] = None
# Add to list of tasks if it is active
if get_active_status(task):
tasks.append(task)
but obviously that doesn't seem to be working as intended.
I'm also confused as to why it would make mpas_analysis fail, rather than global_time_series. The only thing I can think of is the cfg is being parsed incorrectly because the reader is expecting a subtask name. But the .settings files seem to be fine and clearly the dependency chaining is working (global_time_series task is left waiting because mpas_analysis failed).
@xylar have you ever seen that assert(runningProcessCount > 0) error shown above? Do you have any idea if/how that could be connected to the cfg subsections?
@golaz The __many__ part dates back to the very beginning of zppy, the initial commit. Do you recall any oddities/gotchas when you were working on the initial implementation? -- e.g., __many__ requiring that subsections be named?
There was a problem hiding this comment.
@xylar have you ever seen that assert(runningProcessCount > 0) error shown above? Do you have any idea if/how that could be connected to the cfg subsections?
That usually means one of the MPAS-Analysis tasks crashed without raising an error It's not usually that easy to figure out cases like that but you can grep through the logs from MPAS-Analysis to find out which log or logs don't have the execution time printed out to see which task didn't finish. You can point me to the logs if you prefer.
There was a problem hiding this comment.
@forsyth2, if you look earlier in the log file, there is an error message that points you to the task that is failing:
ERROR in task mpasClimatologyOceanAvg. See log file /lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/zppy-gts-split-20250603-try2/v3.LR.historical_0051/post/scripts/../analysis/mpas_analysis/ts_1985-1995_climo_1990-1995/logs/mpasClimatologyOceanAvg.log for details
Running tasks: 51% |##################### | ETA: 0:00:50
[chr-0127:62288:0:62288] Caught signal 7 (Bus error: <unknown si_code>)
This error looks to me like something bad is happening with ncclimo, and I would suspect it's either related to a bad node or running out of disk space. In any case, not obviously anything to do with MPAS-Analysis.
There was a problem hiding this comment.
Thanks @xylar! And wow, yes it appears to have been an intermittent issue like a bad node. I just tried again without an explicit subsection and it worked fine. I guess I just got unlucky with the intermittent issue occurring exactly when I tested that case before...
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
5d97ad0 to
81bbae5
Compare
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
03e32de to
024c568
Compare
|
Results of running the v3 @chengzhuzhang I still want to do some code cleanup and run the full integration test suite, but I have some initial results. At this stage though, can you just 1) confirm each link is the format/style/look we actually want, and 2) confirm if performance speeds are acceptable. Thanks!
|
|
For reference, those are generated with subtasks in |
|
Results from running integration test suite: Summary of test resultsDiff subdir is where to find the lists of missing/mismatched images, the image diff grid, and the individual diffs.
Conclusions:
|
|
Results analysis: Failures are because...
I will rerun with any necessary code fixes. Expected results will be updated after merging this PR (i.e., once we know there are no further changes). |
|
Latest test results: all good. All errors can be accounted for as expected:
Summary of test resultsDiff subdir is where to find the lists of missing/mismatched images, the image diff grid, and the individual diffs.
|
forsyth2
left a comment
forsyth2
left a comment
There was a problem hiding this comment.
@chengzhuzhang This PR and its corresponding zppy-interfaces PR E3SM-Project/zppy-interfaces#26 are ready for review.
This PR is built on top of #720, which forms the first two commits and is focused entirely on testing updates. Once this PR is approved, I think it would be best to merge #720 and then merge this PR (and the corresponding zppy-interfaces PR of course).
As usual, for the review, ignore anything in the generated subdirectory.
tests/integration/image_checker.py
Outdated
|
|
||
| # Make diff_dir readable | ||
| if os.path.exists(parameters.diff_dir): | ||
| os.system(f"chmod -R 755 {parameters.diff_dir}") |
There was a problem hiding this comment.
This might be need to be changed to satisfy the Codacy Static Code Analysis.
| # Draw red box around diff-area on each of: diff, actual, expected | ||
| _draw_box(diff, diff, os.path.join(diff_dir, f"{image_name}_diff.png")) | ||
| _draw_box( | ||
| actual_png, diff, os.path.join(diff_dir, f"{image_name}_actual.png") | ||
| ) | ||
| _draw_box( | ||
| expected_png, diff, os.path.join(diff_dir, f"{image_name}_expected.png") | ||
| ) | ||
|
|
||
|
|
||
| def _draw_box(image, diff, output_path: str): | ||
| # https://stackoverflow.com/questions/41405632/draw-a-rectangle-and-a-text-in-it-using-pil | ||
| draw = ImageDraw.Draw(image) | ||
| (left, upper, right, lower) = ( | ||
| diff.getbbox() | ||
| ) # We specifically want the diff's bounding box | ||
| draw.rectangle(((left, upper), (right, lower)), outline="red") | ||
| image.save(output_path, "PNG") |
There was a problem hiding this comment.
This is a little new (and more properly part of the #720 PR). Previously, the red bounding box was only drawn on the _diff, but here we draw it on the _actual and _expected images as well.
(I haven't actually gotten a mismatched image while running this version of the code, so it's not fully tested. I might try to force a comparison to an obviously different image to check the behavior.)
| input_subdir = "archive/lnd/hist" | ||
| mapping_file = "glb" | ||
| vars = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| vars = "" |
There was a problem hiding this comment.
Now testing with NCO running all lnd variables.
| [[ specific_var_viewers ]] | ||
| # Produce 2 viewers -- an atm one & a lnd one. | ||
| plots_atm = "TREFHT" | ||
| plots_lnd = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| make_viewer = True | ||
|
|
||
| [[ all_lnd_var_viewer ]] | ||
| # Produce 1 viewer, for all lnd variables. | ||
| partition = "compute" | ||
| plots_lnd = "all" | ||
| make_viewer = True | ||
| walltime = "03:00:00" | ||
|
|
||
| [[ classic_original_8_no_ocn ]] | ||
| # Produce original plots minus the ocean ones. | ||
| plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,net_atm_water_imbalance" | ||
|
|
||
| [[ classic_original_8 ]] | ||
| moc_file = "mocTimeSeries_1985-1995.nc" | ||
| # Defaults to: | ||
| # plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,change_ohc,max_moc,change_sea_level,net_atm_water_imbalance", |
There was a problem hiding this comment.
This section replaces the integration tests from the zppy-interfaces repo.
| @@ -0,0 +1,168 @@ | |||
| # Test bundles. | |||
There was a problem hiding this comment.
Legacy cfg files were copied from main as of v3.0.0's release. These ensure there are no backwards compatibility issues. That is, they make sure nothing breaks. Functionality, however, may shift. For instance, setting make_viewer=False (the default) now ignores the component plots (see #722 (comment)).
| ts_years = string_list(default=list("")) | ||
| # `years = "1-100",` would plot years 1 to 100 on the graphs. | ||
|
|
||
| [[__many__]] |
There was a problem hiding this comment.
Adds global_time_series subtask functionality. Per the legacy testing, not having a subsection does not cause a failure.
|
Copied from E3SM-Project/zppy-interfaces#26 (comment): Web results from latest test run |
chengzhuzhang
left a comment
chengzhuzhang
left a comment
There was a problem hiding this comment.
The code change looks good to me.
@forsyth2 do we need to update the example cfg to reflect this change?
It depends what plots you want displayed. If you want both the Classic PDF and land/other component viewers, then yes, we'll need to add subtasks to the |
Yes, the current version of cfg includes both, then before merging, it needs to be updated. |
|
Design decision made: Before merging, we also need to ensure backwards compatibility -- that is, the original plots should still be created even if |
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
fc5a9ab to
b21e36d
Compare
|
Rebased off latest git checkout issue-23-gts-split
git fetch upstream image-checker
# Save backup of the branch
git checkout -b issue-23-gts-split-backup-20250617
git checkout issue-23-gts-split
git rebase upstream/image-checker
# The first 2 of 10 commands require conflicts to be addressed.
# The auto-generated files in particular make this an annoying task, because they often have parameters shifting values.
# At this point, it is probably cleanest (and safest) to simply squash all the commits together
git rebase --abort
# Rebase off the second commit, since the first 2 commits are actually from #720.
git rebase -i d6d5bd05d311e273b30fb5cdbcd6b4c2e299a47e
git fetch upstream image-checker
git rebase upstream/image-checker
# Now, we only have one commit to address conflicts on
# Address conflicts on tests/integration/image_checker.py
git add tests/integration/image_checker.py
git rebase --continue
git push -f upstream issue-23-gts-splitRemaining TODO:
|
git fetch upstream main
git checkout -b test-example-cfg-main upstream/main
# Delete unnecessary blocks from zppy/examples/post.v3.LR.historical.zppy_v3.cfg
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-main-20250617
conda activate zppy-main-20250617
pip install .
zppy -c examples/post.v3.LR.historical.zppy_v3.cfg
cd /lcrc/group/e3sm/ac.forsyth2/zppy_example_v3_20250617_try3/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# No errorsUnderstanding the The web output reflects
Notes:
So, what level of backwards compatibility do we want here?
@chengzhuzhang There are 7 cases to consider for backwards compatibility, that I've grouped together into two real options. Please advise if (A) or (B) is the preferred design decision. Thanks! |
@forsyth2 I tried to compare the behavior change with this PR. Yes, it is preferred to have the original standard 8 panel pdf created even if The I think this would allow backward compatibility. |
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
b21e36d to
7053988
Compare
|
@chengzhuzhang Please see questions in bold. Syncing up the above comments with the cases listed in #722 (comment):
So, we want case 5 to have consistent behavior.
So, we want case 1 to have consistent behavior.
That's cases 2 and 4. There are no issues in this implementation with backwards compatibility for these cases.
So, is the following the expected behavior for component (
Well, that is definitely not backwards compatible -- that's reversing the current default. That means people who were just plotting the Classic PDFs will now see a viewer with a single row "original" that links to the Also, the analysis above still leaves 2 cases unaccounted for:
|
I might use |
|
Results generated here, used the config pasted below, which I thought would create |
This is case 7 above -- Those do actually get created; they just don't get linked in the viewer: You're saying we should have these linked as well? Would that look like 3 rows on the top-level page: "glb_original", "n_original", "s_original"? |
Based on Slack message "my main request is to have the original plots (n,s,glb) created and accessible by default", the answer to this is yes. |
|
Ok, I believe this is the desired design spec based on the 8 possible combinations of parameters:
By default: make_viewer = False, plots_original = 8 plots, plots_ = "". That is: | F | T | F |, or case 6 in the table above. |
|
I've included the above table as in-line documentation over on |
Should that be listed as three extra rows on the viewer home page? E.g., glb_original, n_original, s_original? Or should we in fact create a one-row viewer, e.g., the one row is "Classic PDF" and the columns, as with the components, are "glb, n, s"? |
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
c748990 to
ed9750a
Compare
| [[ viewer_both ]] | ||
| # 1. make_viewer = True, plots_original set, >= 1 plots_<component> set | ||
| # NOTE: This is the case displayed in examples/post.v3.LR.historical_zppy_v3.cfg | ||
| make_viewer = True | ||
| moc_file = "mocTimeSeries_1985-1995.nc" | ||
| plots_atm = "TREFHT" | ||
| plots_lnd = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| # Default plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,change_ohc,max_moc,change_sea_level,net_atm_water_imbalance", | ||
|
|
||
| [[ viewer_original ]] | ||
| # 2. make_viewer = True, plots_original set, 0 plots_<component> set | ||
| make_viewer = True | ||
| moc_file = "mocTimeSeries_1985-1995.nc" | ||
| # Default plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,change_ohc,max_moc,change_sea_level,net_atm_water_imbalance", | ||
|
|
||
| [[ viewer_component ]] | ||
| # 3. make_viewer = True, plots_original set, >= 1 plots_<component> set | ||
| make_viewer = True | ||
| plots_atm = "TREFHT" | ||
| plots_lnd = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| plots_original = "" | ||
|
|
||
| [[ viewer_none ]] | ||
| # 4. make_viewer = True, plots_original not set, 0 plots_<component> set | ||
| make_viewer = True | ||
| plots_original = "" | ||
|
|
||
| [[ classic_both ]] | ||
| # 5. make_viewer = False, plots_original set, >= 1 plots_<component> set | ||
| # Default make_viewer = False | ||
| moc_file = "mocTimeSeries_1985-1995.nc" | ||
| plots_atm = "TREFHT" | ||
| plots_lnd = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| # Default plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,change_ohc,max_moc,change_sea_level,net_atm_water_imbalance", | ||
|
|
||
| [[ classic_original ]] | ||
| # 6. make_viewer = False, plots_original set, 0 plots_<component> set | ||
| # NOTE: This is the default case -- where all of the above parameters are at their default values. | ||
| # Default make_viewer = False | ||
| moc_file = "mocTimeSeries_1985-1995.nc" | ||
| # Default plots_original = "net_toa_flux_restom,global_surface_air_temperature,toa_radiation,net_atm_energy_imbalance,change_ohc,max_moc,change_sea_level,net_atm_water_imbalance", | ||
|
|
||
| [[ classic_component ]] | ||
| # 7. make_viewer = False, plots_original set, >= 1 plots_<component> set | ||
| # Default make_viewer = False | ||
| plots_atm = "TREFHT" | ||
| plots_lnd = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR" | ||
| plots_original = "" | ||
|
|
||
| [[ classic_none ]] | ||
| # 8. make_viewer = False, plots_original not set, 0 plots_<component> set | ||
| # Default make_viewer = False | ||
| plots_original = "" |
There was a problem hiding this comment.
These 8 cases cover the relevant parameter combinations.
Understanding changes in design decisions over time
The original design decisions were:
- We want the Classic PDF to be pretty standard; don’t anticipate adding arbitrary variables.
- No viewer needed for the classic PDF
From #722 (comment):
the
make_viewerparameter can control if a sub-viewer is created or an aggregated pdf file for all variables should be created (the latter should be a less popular setting in case of an extensive variable list like those complete land variables, somake_viewertrue should be the default).
This design spec revision undoes note (1) above, and thus we now need to test cases 5 (classic_both) and 7 (classic_component).
From the same comment:
it is preferred to have the original standard 8 panel pdf created even if make_viewer=True
The original row always display and links to the standard 8 panel pdf.
This design spec revision undoes note (2) above, and thus we now need to test cases 1 (viewer_both) and 2 (viewer_original).
Before these design spec revisions (which proved necessary for backwards compatibility), we really only needed cases 3 (viewer_component) and 6 (classic_original). (The remaining cases are null input tests).
That is, the original design was for a 1:1 correspondence of viewer mode = we're plotting components, classic PDF mode = we're plotting the original plots. The design spec update means the output type and the plot type are no longer tied together. It is for this reason that the major redesign in zppy-interfaces (E3SM-Project/zppy-interfaces@7752b45) was necessary.
There was a problem hiding this comment.
After including the fixes of E3SM-Project/zppy-interfaces@27c4900, 5 of the cases don't error out and 3 do.
The results can be seen here
- 1. viewer_both -- FAIL, does not construct viewer
- 2. viewer_original -- FAIL, does not construct viewer
- 3. viewer_component -- FAIL, does not construct viewer
- 4. viewer_none -- PASS no output, as expected
- ERROR:
RuntimeError: You must first insert a group with add_group() - ERROR:
rsync: link_stat "/lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/test-gts-split-20250619/v3.LR.historical_0051/post/scripts/global_time_series_viewer_none_1985-1995_results" failed: No such file or directory (2)
- ERROR:
- 5. classic_both -- FAIL, while the Classic PDFs are created for the original plots, and the component plots do in fact get cumulative PDFs, their cumulative PDFs contain only empty plots. Additionally, it looks like empty cumulative PDFs got made even for ocn, ice.
- 6. classic_original -- FAIL, output seems to be no different from classic_both.
- 7. classic_component -- FAIL, no output whatsoever
- ERROR:
rsync: link_stat "/lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/test-gts-split-20250619/v3.LR.historical_0051/post/scripts/global_time_series_cla\ ssic_component_1985-1995_results" failed: No such file or directory (2)
- ERROR:
- 8. classic_none -- PASS, no output, as expected
- ERROR:
rsync: link_stat "/lcrc/group/e3sm/ac.forsyth2/zppy_weekly_comprehensive_v3_output/test-gts-split-20250619/v3.LR.historical_0051/post/scripts/global_time_series_cla\ ssic_none_1985-1995_results" failed: No such file or directory (2)
- ERROR:
Clearly, more work needs to be done on the redesign. (Or else the separation of PDF and Viewer functionality should be scrapped entirely, and performance improvements should be considered for the code as it exists on main.)
|
From latest full test using ZI as of E3SM-Project/zppy-interfaces@733854d Summary of test resultsDiff subdir is where to find the lists of missing/mismatched images, the image diff grid, and the individual diffs.
All of the legacy The only failures are at https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v3_www/test-gts-split-20250630/v3.LR.historical_0051/image_check_failures_comprehensive_v3/global_time_series/missing_images.txt: This is expected, because we now have multiple subtasks, with different subdirectories for this particular test. That said, there is one remaining problem I see: if |
|
Latest test results: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_weekly_comprehensive_v3_www/test-gts-split-20250701/v3.LR.historical_0051/global_time_series/ Summary of test resultsDiff subdir is where to find the lists of missing/mismatched images, the image diff grid, and the individual diffs.
Same as above (#722 (comment)) except the change in |
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
ed9750a to
a301340
Compare
|
@forsyth2 I need to take some time to revisit this PR, there are quite a few possible use cases to consider. I'd like to test it with two of my config files: one for a historical run and another for an AMIP run. Could you remind me how to install both the zppy and zppy-interface branches locally for testing? |
Yes, these are covered by the test cases in the test cfg Full list of test cases
lcrc_conda # Activate your conda; this is my function to do so
# zppy-interfaces side
cd ~/ez/zppy-interfaces
git status
# Make sure there are no changes that might get overwritten
# That is, look for "Your branch is up to date"
git remote -v # See what points to [email protected]:E3SM-Project/zppy-interfaces.git
git fetch upstream # For me, that's upstream
git checkout -b issue-23-gts-split upstream/issue-23-gts-split # Get latest code
git log
# Should have the 4 commits of https://github.com/E3SM-Project/zppy-interfaces/pull/26/commits
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zi-issue-23-20250902
conda activate zi-issue-23-20250902
pre-commit run --all-files # Only if you've made changes
python -m pip install .
pytest tests/unit/global_time_series/test_*.py # Only if you want to double check unit tests
# zppy side
cd ~/ez/zppy
git status
# Make sure there are no changes that might get overwritten
# That is, look for "Your branch is up to date"
git remote -v # See what points to [email protected]:E3SM-Project/zppy.git
git fetch upstream # For me, that's upstream
git checkout -b issue-23-gts-split upstream/issue-23-gts-split # Get latest code
git log
# Should have the 2 commits of https://github.com/E3SM-Project/zppy/pull/722/commits
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-issue-23-20250902
conda activate zppy-issue-23-20250902
pre-commit run --all-files # Only if you've made changes
python -m pip install .
pytest tests/test_*.py # Only if you want to double check unit tests
zppy -c your_configuration_file.cfgLastly, I do see that there are some merge conflicts on the zppy-interfaces side, so I will take a look at resolving those & rebasing off the latest |
These are just because of the logger import line changes from E3SM-Project/zppy-interfaces#29. So nothing big, just annoying because it touches every file. I will try work on getting those resolved, but in the meantime, they shouldn't affect your testing (other than the fact logger statements will still print multiple times). |
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
a301340 to
d19c828
Compare
forsyth2
force-pushed
the
issue-23-gts-split
branch
from
5771d34 to
69c0a18
Compare
Summary
Corresponding
zppy-interfacesPR: E3SM-Project/zppy-interfaces#26Objectives:
zppy-interfaces code, as of Extension of work on Split global_time_series task and improve performance zppy-interfaces#26Issue resolution:
Select one: This pull request is...
Big Change
1. Does this do what we want it to do?
Required:
If applicable:
2. Are the implementation details accurate & efficient?
Required:
If applicable:
zppy/conda, not just animportstatement.3. Is this well documented?
Required:
4. Is this code clean?
Required:
If applicable: