Skip to content

Automatically determine diags paths #299

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

Closed
forsyth2 opened this issue Sep 1, 2022 · 8 comments · Fixed by #708
Closed

Automatically determine diags paths #299

forsyth2 opened this issue Sep 1, 2022 · 8 comments · Fixed by #708
Labels
priority: low Low priority task

Comments

@forsyth2
Copy link
Collaborator

forsyth2 commented Sep 1, 2022

The code block at https://github.com/E3SM-Project/zppy/blob/main/zppy/e3sm_diags.py#L68 already allows for reference paths to be "guessed" from the main reference path given. This could be extended further -- for example, dc_obs_climo and ts_obs could similarly be "guessed" from the observation paths.

@forsyth2 forsyth2 added the priority: low Low priority task label Sep 1, 2022
@chengzhuzhang
Copy link
Collaborator

Bump up this issue, when finalizing the cfg file for v3, @golaz has suggested that we can make the .cfg files more portable by using mache. Mache has already used for mapping files, we can use it to auto provide machine-specific diags paths as well.

@forsyth2
Copy link
Collaborator Author

forsyth2 commented Apr 4, 2025

@chengzhuzhang This might already be resolved...

In the latest https://github.com/E3SM-Project/zppy/blob/main/zppy/e3sm_diags.py, we have the parameter inference feature to determine dc_obs_climo and tc_obs.

        if "diurnal_cycle" in c["sets"]:
            set_value_of_parameter_if_undefined(
                c,
                "dc_obs_climo",
                c["reference_data_path"],
                ParameterInferenceType.PATH_INFERENCE,
            )
    if "tc_analysis" in c["sets"]:
        set_value_of_parameter_if_undefined(
            c,
            "tc_obs",
            f"{c['diagnostics_base_path']}/observations/Atm/tc-analysis/",
            ParameterInferenceType.PATH_INFERENCE,
        )

The parameter inference feature was very extensive; I think it should cover many use cases.

@chengzhuzhang
Copy link
Collaborator

Oh, I didn't know. I think next step is to use it in .cfg to test if they work as intended.

@forsyth2
Copy link
Collaborator Author

forsyth2 commented Apr 4, 2025

@chengzhuzhang Just note that some of @golaz's suggested use cases are beyond zppy's inference capabilities -- e.g., #694 (comment), hence they require explicit parameter setting.

@chengzhuzhang
Copy link
Collaborator

I looked at the inference code. It looks like reference_data_path can be inferred:
https://github.com/E3SM-Project/zppy/blob/bfbba5f9c3794cd7e399e35f8153866b1c8f6910/zppy/e3sm_diags.py#L167C5-L172C6 did i miss understand?

@chengzhuzhang
Copy link
Collaborator

in my current attempt, i removed all the reference data path in the config, and to see if zppy can find create the correct paths.

@forsyth2
Copy link
Collaborator Author

forsyth2 commented Apr 4, 2025

It looks like reference_data_path can be inferred:

Yes, it can be inferred, but an inference isn't a guarantee. It works fine if the reference data is where zppy thinks it will be. So, the inference will work if the reference data actually is at f"{c['diagnostics_base_path']}/observations/Atm/climatology/". If it's organized in any other way, say we actually want data at f"{c['diagnostics_base_path']}/observations_new_data/Atm/climatology/", then it's going to be a bad inference.

@chengzhuzhang
Copy link
Collaborator

okay, got it. I think at this point we just need the standard paths, without needing to consider the edge cases mentioned. It looks like the bash file looks correct. I will update once the runs are finished. And in cases user needs to use a new obs path or they are not on a mache supported machines, they just need to manually add the paths parameters to point to the data.

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

Successfully merging a pull request may close this issue.

2 participants