Latest NCO removes 3D vars #717
Conversation
forsyth2
added
the
semver: small improvement
python tests/integration/utils.py
pip install .
# This cfg tests vars="" on the ts lnd task.
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_original_atm_plus_land_chrysalis.cfg
cd /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_output/test_704/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# The ts lnd tasks hit the time limitThe new NCO seems to be hitting the time limit; will need to check if that happens on |
|
Please keep me posted. Nothing I changed in NCO should be taking longer. If it turns out that NCO is hanging or taking longer on the ts lnd task, then it would be helpful to receive a sample command that demonstrates this. |
|
I ran the same block as above, but on So what's different here? Not much -- the relevant diff is pretty minimal: What about how it's called? In both cases, I'm running That is, we're asking for all available land variables. On |
|
Re-running on this branch and it does seem to be taking longer, so unfortunately I don't think it was an anomaly the other day. |
|
Just to confirm, have you verified that using |
|
@czender yeah, that's a good idea to check, thanks. I'm not going to have time to check today, but hopefully tomorrow. |
|
@czender I just ran with |
|
Thank for running the short test. I'll look at this output tomorrow and see if I can reproduce and then fix this behavior. |
|
Please send me the full absolute path to the land input files used in this test |
|
input: relevant excerpt of # Create symbolic links to input files
input=/lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051/archive/lnd/hist
for (( year=1985; year<=1989; year++ ))
do
YYYY=`printf "%04d" ${year}`
for file in ${input}/v3.LR.historical_0051.elm.h0.${YYYY}-*.nc
do
ln -s ${file} .
done
done
vars=TBOT,QBOT,W_SCALAR
# https://unix.stackexchange.com/questions/237297/the-fastest-way-to-remove-a-string-in-a-variable
# https://stackoverflow.com/questions/26457052/remove-a-substring-from-a-bash-variable
# Remove U, since it is a 3D variable and thus will not work with rgn_avg
vars=${vars//,U}
ls v3.LR.historical_0051.elm.h0.????-*.nc > input.txt
if grep -q "*" input.txt; then
cd /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_output/test_704_20250529_minimal_vars/v3.LR.historical_0051/post/scripts
echo 'Missing input files'
echo 'ERROR (1)' > ts_lnd_monthly_glb_1985-1989-0005.status
exit 1
fi
# Generate time series files
# If the user-defined parameter "vars" is "", then ${vars}, defined above, will be too.
cat input.txt | /home/ac.zender/bin_chrysalis/ncclimo --npo \
-c v3.LR.historical_0051 \
-v ${vars} \
--split \
--yr_srt=1985 \
--yr_end=1989 \
--ypf=5 \
-o output \
--rgn_avg \
--area=area \
--prc_typ=elmoutput: cfg excerpt: Full cfg |
|
Sorry for the delay. I can reproduce this behavior. It looks like the 3D variables are not being discarded as I thought they would be. Definitely an issue on my end. Hope to fix by early next week... |
|
@forsyth2 I spoke too soon. The code is expected to fail when the user explicitly requests a global mean timeseries of a 3D variable. This is in keeping with the general NCO philosophy of failing when asked to do what cannot be done. So it fails when |
|
|
FYI I released NCO 5.3.4 which is what the |
|
@czender Thanks for the continued work here! Unfortunately, I have several other priorities at the moment, so it may be a bit before I can get back to this. Will give an update when I can. |
|
@czender Sorry, I finally had some time to get back to this, but now I can't seem to get your custom NCO to work at all. Command line, no conda, `bin_chyrsalis/ncclimo`# https://github.com/E3SM-Project/zppy/pull/717#issuecomment-2951125003
test_num=2
caseid=v3.LR.historical_0051
drc_in=/lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051/archive/lnd/hist
drc_out=/lcrc/group/e3sm/ac.forsyth2/issue_704_test${test_num}
cd ${drc_in}
ls ${caseid}.elm.h0.198[5-9]-??.nc ${caseid}.elm.h0.199[0-5]-??.nc | /home/ac.zender/bin_chrysalis/ncclimo --npo -c v3.LR.historical_0051 --split --yr_srt=1985 --yr_end=1995 --ypf=5 -o ${drc_out} --rgn_avg --area=area --prc_typ=elmgives: Command line, Unified, `bin_chyrsalis/ncclimo`Same script as above gives many iterations of That seems to me to be an error _inside _ Running zppy, Unified, `bin_chyrsalis/ncclimo`Cfg: zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_original_atm_plus_land_chrysalis.cfggives: I've seen this |
|
Thanks for the info, @forsyth2. You are running the bleeding edge NCO, and it appears you've found an ncclimo syntax error. I'll look into this soon and let you know when I've made the necessary updates. |
|
@czender I'm helping @thorntonpe to create global time series plots for all 2d land variables and looks like I need to use your latest snapshot for generating global metrics, without manually removing 3d vars. Not sure if the latest nco image on LCRC is implemented with this feature? |
Hey @czender, just checking in if you've had a chance to look into this yet. Thanks! |
|
@forsyth2 I'm looking into it now. I have an idea as to what may be causing the issues you report. I'll try it tommorrow. |
|
I'm making progress on this, and will have a new implementation you can try tomorrow or Friday. |
|
Great, thanks @czender! |
|
@forsyth2 I re-factored the |
Test 3 ( cd ~/ez/zppy
lcrc_conda # Function to activate conda
conda clean --all --y
conda env create -f conda/dev.yml -n zppy-704-20250904
conda activate zppy-704-20250904
# Call ncclimo directly, with Unified environment
# 2nd test case from 7/23 run
unified # Alias to set up unified environment
test_num=1
caseid=v3.LR.historical_0051
drc_in=/lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051/archive/lnd/hist
drc_out=/lcrc/group/e3sm/ac.forsyth2/issue_704_20250904test${test_num}
cd ${drc_in}
ls ${caseid}.elm.h0.198[5-9]-??.nc ${caseid}.elm.h0.199[0-5]-??.nc | /home/ac.zender/bin_chrysalis/ncclimo --npo -c v3.LR.historical_0051 --split --yr_srt=1985 --yr_end=1995 --ypf=5 -o ${drc_out} --rgn_avg --area=area --prc_typ=elm
# syntax error near unexpected token `)'
# Call ncclimo directly, without Unified environment
# 1st test case from 7/23 run
lcrc_conda
test_num=2
drc_out=/lcrc/group/e3sm/ac.forsyth2/issue_704_20250904test${test_num}
ls ${caseid}.elm.h0.198[5-9]-??.nc ${caseid}.elm.h0.199[0-5]-??.nc | /home/ac.zender/bin_chrysalis/ncclimo --npo -c v3.LR.historical_0051 --split --yr_srt=1985 --yr_end=1995 --ypf=5 -o ${drc_out} --rgn_avg --area=area --prc_typ=elm
# ncks: error while loading shared libraries: libgsl.so.28: cannot open shared object file: No such file or directory
# Call ncclimo via zppy
# 3rd test case from 7/23 run
cd ~/ez/zppy
conda activate zppy-704-20250904
git log
# Commits:
# Testing
# Updates
# Latest NCO removes 3D vars (only commit currently pushed to GitHub)
# Merge pull request #723 from E3SM-Project/fix-pm-tc-analysis
# Edit tests/intgeration/utils.py
# UNIQUE_ID = "test_pr717_20250904"
# "diags_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh",
# "global_time_series_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate zi-main-20250822",
python tests/integration/utils.py
python -m pip install .
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_original_atm_plus_land_chrysalis.cfg
cd /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_output/test_pr717_20250904/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# No errors
cd ~/ez/zppy
emacs zppy/templates/ts.bash
# Uses latest NCO
# /home/ac.zender/bin_chrysalis/ncclimo --npo |
|
@czender It's possible I'm just not calling Also, I suppose we'll again have the situation of needing to remove the |
|
Thanks for re-testing. That's good progress. Unfortunately the nature of the |
|
@forsyth2 I just noticed that the failure in your test #1 changed from not finding the right |
|
@czender Ah that fixes it, looks like both (1) and (3) work now. # 1st test case from 7/23 run
lcrc_conda
gsl-config --prefix
# -bash: gsl-config: command not found
# 2nd test case from 7/23 run
unified
gsl-config --prefix
# /lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.1_login
# 3rd test case from 7/23 run
lcrc_conda
conda activate zppy-704-20250904
gsl-config --prefix
# -bash: gsl-config: command not foundand # Call ncclimo directly, without Unified environment
# 1st test case from 7/23 run
lcrc_conda
test_num=2
caseid=v3.LR.historical_0051
drc_in=/lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051/archive/lnd/hist
drc_out=/lcrc/group/e3sm/ac.forsyth2/issue_704_20250905test${test_num}
cd ${drc_in}
ls ${caseid}.elm.h0.198[5-9]-??.nc ${caseid}.elm.h0.199[0-5]-??.nc | /home/ac.zender/bin_chrysalis/ncclimo --npo -c v3.LR.historical_0051 --split --yr_srt=1985 --yr_end=1995 --ypf=5 -o ${drc_out} --rgn_avg --area=area --prc_typ=elm
# 12m41s runtime, success!
# Call ncclimo via zppy
# 3rd test case from 7/23 run
cd ~/ez/zppy
git checkout issue-704
conda activate zppy-704-20250904
# Edit tests/intgeration/utils.py
# UNIQUE_ID = "test_pr717_20250905"
# "diags_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh",
# "global_time_series_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate zi-main-20250822",
python tests/integration/utils.py
python -m pip install .
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_original_atm_plus_land_chrysalis.cfg
cd /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_output/test_pr717_20250905/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# No errors, success!
cd ~/ez/zppy
emacs zppy/templates/ts.bash
# Uses latest NCO
# /home/ac.zender/bin_chrysalis/ncclimo --npo |
|
Hmm, that is interesting. I've seen |
|
Hmm ran test case 1 again, still ran just fine. |
|
@czender Can I merge this or are you concerned about test 1? Test 3 is the important use case here, being as it actually uses |
|
I would say run the test one more time. If the glitch with #1 does not repeat then merge it. However, know that there have been other changes to NCO in the interim, so there could be a failure due to unrelated causes. |
|
Looks like both test cases are still working: # Call ncclimo directly, without Unified environment
# 1st test case from 7/23 run
lcrc_conda
test_num=3
caseid=v3.LR.historical_0051
drc_in=/lcrc/group/e3sm2/ac.wlin//E3SMv3/v3.LR.historical_0051/archive/lnd/hist
drc_out=/lcrc/group/e3sm/ac.forsyth2/issue_704_20250905test${test_num}
cd ${drc_in}
ls ${caseid}.elm.h0.198[5-9]-??.nc ${caseid}.elm.h0.199[0-5]-??.nc | /home/ac.zender/bin_chrysalis/ncclimo --npo -c v3.LR.historical_0051 --split --yr_srt=1985 --yr_end=1995 --ypf=5 -o ${drc_out} --rgn_avg --area=area --prc_typ=elm
# Elapsed time 14m36s
# Success
# Call ncclimo via zppy
# 3rd test case from 7/23 run
cd ~/ez/zppy
git status
# Check for uncommitted changes
git checkout issue-704
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zstash-nco-20250916
conda activate zstash-nco-20250916
git fetch upstream
git rebase upstream/main
git log # Includes latest changes on `main` branch
# Edit tests/intgeration/utils.py
# TEST_SPECIFICS: Dict[str, Any] = {
# "diags_environment_commands": "source /lcrc/soft/climate/e3sm-unified/load_latest_e3sm_unified_chrysalis.sh",
# "global_time_series_environment_commands": "source /gpfs/fs1/home/ac.forsyth2/miniforge3/etc/profile.d/conda.sh; conda activate zi-main-20250822",
# "cfgs_to_run": [
# "min_case_global_time_series_viewers_original_atm_plus_land",
# ],
# "tasks_to_run": ["e3sm_diags", "mpas_analysis", "global_time_series", "ilamb"],
# "unique_id": "test_pr717_20250916",
# }
python tests/integration/utils.py
python -m pip install .
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_original_atm_plus_land_chrysalis.cfg
cd /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_output/test_pr717_20250916/v3.LR.historical_0051/post/scripts
grep -v "OK" *status
# No errors, success!
cd ~/ez/zppy
emacs zppy/templates/ts.bash
# Good, uses latest NCO:
# /home/ac.zender/bin_chrysalis/ncclimo --npo |
|
Based on the above testing, I'm going to merge. |
Summary
Objectives:
vars_excludeparameter, labeling it as deprecated, and adding it to the deprecated parameters test (i.e., test that it has no effect)Issue resolution:
Select one: This pull request is...
Please fill out either the "Small Change" or "Big Change" section (the latter includes the numbered subsections), and delete the other.
Small Change