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

Delete adf_quick_run.ipynb now that link_to_ADF.ipynb exists #198

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TeaganKing
Copy link
Collaborator

This PR removes the old adf_quick_run notebook since we now prefer supporting link_to_ADF.ipynb and the helper scripts that create the ADF configuration files

All PRs Checklist:

@mnlevy1981
Copy link
Collaborator

adf_quick_run.ipynb is still run from examples/coupled_model/config.yml:

    atm:
      adf_quick_run:
        parameter_groups:
          none:
            adf_path: ../../../externals/ADF
            config_path: .
            config_fil_str: "config_f.cam6_3_119.FLTHIST_ne30.r328_gamma0.33_soae.001.yaml"

We've talked about encouraging folks to use key_metrics instead of coupled_model, but until we get rid of the latter (which should be done, since it doesn't follow the same variable name conventions as key_metrics and that is confusing) I think we should keep the ADF quick run notebook.

Another concern is that maybe @justin-richling, @brianpm, or @nusbaume want to provide an example of invoking ADF from a notebook instead of the command line. If that's not the case, maybe we could remove the quick run notebook but run a different atmosphere notebook in coupled_model?

Copy link
Collaborator

@mnlevy1981 mnlevy1981 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should've marked "request changes" with #198 (comment)

@nusbaume
Copy link
Collaborator

nusbaume commented Mar 6, 2025

Hi @mnlevy1981 @TeaganKing, after discussing it in an ADF meeting we decided that there isn't really a need to have an example of the ADF being called from a notebook (calling it directly in python or from the command line is fine with us), so feel free to move ahead with this particular PR. Thanks for checking!

@TeaganKing
Copy link
Collaborator Author

Thanks @nusbaume for this feedback. @mnlevy1981 I removed this from the coupled model config file, too. That said, I'm not in any rush to get this in, and it could certainly be done as a larger removal (or updating) of the coupled model example.

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 this pull request may close these issues.

3 participants