Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For model vs model e3sm_diags runs incorrect time series path for test data #471

Closed
chengzhuzhang opened this issue Aug 4, 2023 · 1 comment · Fixed by #472
Closed

Comments

@chengzhuzhang
Copy link
Collaborator

chengzhuzhang commented Aug 4, 2023

Reported by @golaz, in the qbo model vs model results, test and reference run appears using the same data (reference run data). Same applied to other time series based analysis, such as enso diags. The run is configured with zppy, I suspect the e3sm_diags template has a bug to assign test data path.

image

Command line to reproduce:
Screen Shot 2023-08-03 at 5 28 00 PM

@forsyth2
Copy link
Collaborator

forsyth2 commented Aug 4, 2023

Debugging

The following is in the main body of the script:

{%- if "qbo" in sets %}                                                                                                                                  
qbo_param = QboParameter()                                                                                                                               
qbo_param.test_data_path = test_ts                                                                                                                       
qbo_param.test_name = short_name                                                                                                                         
qbo_param.test_start_yr = start_yr                                                                                                                       
qbo_param.test_end_yr = end_yr                                                                                                                           
qbo_param.ref_start_yr = ref_start_yr                                                                                                                    
ref_end_yr = ref_start_yr + num_years - 1                                                                                                                
if (ref_end_yr <= {{ ref_final_yr }}):                                                                                                                   
  qbo_param.ref_end_yr = ref_end_yr                                                                                                                      
else:                                                                                                                                                    
  qbo_param.ref_end_yr = {{ ref_final_yr }}                                                                                                              
{% if run_type == "model_vs_obs" %}                                                                                                                      
# Obs                                                                                                                                                    
qbo_param.reference_data_path = '{{ obs_ts }}'                                                                                                           
{% elif run_type == "model_vs_model" %}                                                                                                                  
# Reference                                                                                                                                              
qbo_param.reference_data_path = '${ts_dir_ref}'                                                                                                          
qbo_param.ref_name = '{{ ref_name }}'                                                                                                                    
qbo_param.short_ref_name = '{{ short_ref_name }}'                                                                                                        
# Optionally, swap test and reference model                                                                                                              
if {{ swap_test_ref }}:                                                                                                                                  
   qbo_param.test_data_path, qbo_param.reference_data_path = qbo_param.reference_data_path, qbo_param.test_data_path                                     
   qbo_param.test_name, qbo_param.ref_name = qbo_param.ref_name, qbo_param.test_name                                                                     
   qbo_param.short_test_name, qbo_param.short_ref_name = qbo_param.short_ref_name, qbo_param.short_test_name                                             
{%- endif %}

test_ts = '${ts_dir_destination}' in the main body.

ts_dir_destination=$2 is in create_links_ts()

create_links_ts ${ts_dir_source} ${ts_dir_primary} ${Y1} ${Y2} 5 in the main body

create_links_ts ${ts_dir_source} ${ts_dir_ref} ${ref_Y1} ${ref_Y2} 6 in the main body (only called if run_type == "model_vs_model")

So, what's happening is ts_dir_destination keeps its last used value (${ts_dir_ref} rather than ${ts_dir_primary}).

It's not a good programming practice to use a variable defined in a function outside that function (and in many languages other than bash, that wouldn't have even worked because variables are appropriately scoped). It looks like originally, ts_dir_destination (or the equivalent value with a different name) was defined in the main body. Then, in #195, I created a function so code wouldn't get duplicated. I inadvertently placed the definition of ts_dir_destination into that function

ts_dir_ref=ts_ref in the main body, so that's where test_data_path = 'ts_ref' comes from in the image above.

As for ts_dir_primary, the following is in the main body:

{% if run_type == "model_vs_obs" %}
ts_dir_primary=ts
{% elif run_type == "model_vs_model" %}
ts_dir_primary=ts_test

So, I think we just need to replace test_ts = '${ts_dir_destination}' with test_ts = '${ts_dir_primary}'. That would make qbo_param.test_data_path = test_ts actually differ from qbo_param.reference_data_path = '${ts_dir_ref}'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants