Counterintuitive "reference_data_path" in model-vs-model

[mahf708]

I encountered what I thought was counterintuitive behavior in the following code (also quoted below for convenience).

By asking for reference_data_path in a model-vs-model block, a noob like me might think the actual .nc data, e.g., ${CASE_ROOT}/post/atm/180x360_aave/clim/ for the [[ atm_monthly_180x360_aave ]] block. It turns out this noob tendency was partly anticipated by the developers and was handled by the code by trimming at /post (and adding it again). However, this will fail if the grid is not supplied — which in this noob's case, it was definitely missing 😵‍💫

Anyway, this got me to make a gentle low-priority suggestion: It could be more informative to rename this variable (while keeping backward compatibility) as something like refernece_model_path or reference_case_path for the model-vs-model cases.

Thank you for the great work!

            elif c["run_type"] == "model_vs_model":
                prefix = "e3sm_diags_%s_%s_%04d-%04d_vs_%04d-%04d" % (
                    c["sub"],
                    c["tag"],
                    c["year1"],
                    c["year2"],
                    c["ref_year1"],
                    c["ref_year2"],
                )
                reference_data_path = (
                    c["reference_data_path"].split("/post")[0] + "/post"
                )
                if ("diurnal_cycle" in c["sets"]) and (
                    c["reference_data_path_climo_diurnal"] == ""
                ):
                    c[
                        "reference_data_path_climo_diurnal"
                    ] = f"{reference_data_path}/atm/{c['grid']}/clim_diurnal_8xdaily"
                if ("tc_analysis" in c["sets"]) and (c["reference_data_path_tc"] == ""):
                    c[
                        "reference_data_path_tc"
                    ] = f"{reference_data_path}/atm/tc-analysis_{c['ref_year1']}_{c['ref_year2']}"
                if ("ts_num_years" in c.keys()) and (c["reference_data_path_ts"] == ""):
                    c[
                        "reference_data_path_ts"
                    ] = f"{reference_data_path}/atm/{c['grid']}/ts/monthly"
                if ("streamflow" in c["sets"]) and (
                    c["reference_data_path_ts_rof"] == ""
                ):
                    c[
                        "reference_data_path_ts_rof"
                    ] = f"{reference_data_path}/rof/native/ts/monthly"
                if c["gauges_path"] == "":
                    gauges_path_prefix = c["diagnostics_base_path"]
                    gauges_path_suffix = "observations/Atm/time-series/GSIM/GSIM_catchment_characteristics_all_1km2.csv"
                    c["gauges_path"] = os.path.join(
                        gauges_path_prefix, gauges_path_suffix
                    )

[mahf708]

Also, it may worthwhile to consider inferring the name from the block itself anyway, right? However, I am not sure if there are any conflicting issues or complications to do with that.

[mahf708]

I believe your confusion is how reference_data_path gets used in model-vs-model runs

Yes, I am only concerned with model-vs-model in this case. I will edit the title to make it clearer.

I'm not entirely sure that's a related issue. Does it run if you don't supply grid but do cut off reference_data_path at /post? I believe grid is almost always required.

It errors if grid is not supplied at some point. It errors exactly with missing the grid block in the code I shared.

you're implying we can infer reference_data_path="/lcrc/group/e3sm/ac.forsyth2/zppy_test_complete_run_output/v2.LR.historical_0201/post/atm/180x360_aave/clim" from the block name [[ atm_monthly_180x360_aave_mvm ]]? Especially for model-vs-model, I don't see a way for zppy to know what the user wants to use as reference data.

Yes, so take the middle of the block name (180x360_aave) and take it as a grid name, but there might be other related issues. Again, this is only in the model-vs-model.

I guess another way to say this, it might worth calling reference_data_path something different in the case of a model-vs-model, unlike model-vs-obs. In the model-vs-model, you're comparing the cases or models, but in the case of model-vs-obs, you're comparing the data thereof. At least that's from the code...

[mahf708]

https://github.com/E3SM-Project/zppy/blob/main/tests/integration/generated/test_complete_run_chrysalis.cfg#L120: reference_data_path = "/lcrc/group/e3sm/ac.forsyth2/zppy_test_complete_run_output/v2.LR.historical_0201/post/atm/180x360_aave/clim"

This works because grid is provided earlier:

zppy/tests/integration/generated/test_complete_run_chrysalis.cfg

Line 76 in c3f70dc

grid = '180x360_aave'

[forsyth2]

so take the middle of the block name (180x360_aave) and take it as a grid name

There still wouldn't be a way to know if /lcrc/group/e3sm/ac.forsyth2/zppy_test_complete_run_output/v2.LR.historical_0201/post/ is the right prefix -- perhaps the user wants to compare to a model in someone else's directory.

it might worth calling reference_data_path something different in the case of a model-vs-model, unlike model-vs-obs

I can make an issue to create a new parameter that feeds into reference_data_path in the model-vs-model case.

[mahf708]

No need to act on this right now :D

This was just a low-priority suggestion to consider in the future, nothing urgent. Just reporting user feedback, that's all :)

[forsyth2]

Sounds good, thanks. I created #405 to this.