search

E3SM-Project / zppy

E3SM post-processing toolchain
BSD 3-Clause "New" or "Revised" License
6 stars 15 forks source link

Remove climo_diurnal_input_files internal parameter #612

Closed forsyth2 closed 4 months ago

forsyth2 commented 4 months ago

Remove climo_diurnal_input_files internal parameter. Resolves #607

forsyth2 commented 4 months ago

Ran the following minimal cases (see #608):

e3sm_diags_diurnal_cycle
e3sm_diags_diurnal_cycle_mvm_1
e3sm_diags_diurnal_cycle_mvm_2

These produce results both with and without explicitly setting {{ climo_diurnal_input_files }} or {{ climo_diurnal_input_files_ref }}.

@chengzhuzhang Is it better 1) to be explicit about the input files we're looking for (and thus require an extra parameter) or 2) to have zppy automatically determine the h number? (2) appears to work, but I'm concerned I just found a simple case where that happens to work.

chengzhuzhang commented 4 months ago

@forsyth2 thank you for working on this. To be explicit about the input files would be fine, I'm curious about the mechanism for zppy to automatically infer the h number?

forsyth2 commented 4 months ago

I'm curious about the mechanism for zppy to automatically infer the h number?

The second commit currently in this PR: https://github.com/E3SM-Project/zppy/pull/612/commits/b116dcfd76618e3855e3cd4519ac0e256020e5b4

cp -s ${climo_diurnal_dir_source}/${nc_prefix}.{{ climo_diurnal_input_files_ref }}_*_${begin_year}??_${end_year}??_climo.nc .
cp -s ${climo_diurnal_dir_source}/${nc_prefix}.{{ climo_diurnal_input_files }}_*_${begin_year}??_${end_year}??_climo.nc .

could simply be replaced with

cp -s ${climo_diurnal_dir_source}/${nc_prefix}.*_*_${begin_year}??_${end_year}??_climo.nc .

But my concern is that the directories I'm working with just happen to have only the correct h number in them.

chengzhuzhang commented 4 months ago

@forsyth2 I looked it up e3sm_diags code, I think as long as the the ref directory includes those seasonal mean diurnal cycle files, the h number doesn't matter. The file name just needs to start with the case name. We need to make sure the ref directory should include the correct symlinks.

forsyth2 commented 4 months ago

@chengzhuzhang I've updated the second commit (dcb9eb175ab51090cd1bf69febeb405224b084ca) and it looks like everything still works if we remove the input file specification in all cases for diurnal cycle. It looks like it was originally introduced in https://github.com/E3SM-Project/zppy/commit/aaaa61df13a41124c5b19ec3b85c011ccd2eb6c1#diff-ed02c5b26bf8ed8e97148b4f81cfdca9afc1706855f5a7aba9e0b7f3c47cee52. It seems the cleaner solution would have been to just use * in the first place rather than introducing a parameter.

forsyth2 commented 4 months ago

I've tested with:

chengzhuzhang commented 4 months ago

It seems the cleaner solution would have been to just use * in the first place rather than introducing a parameter.

Yes, I think it is a better solution.