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

PR for Issue #89: "Ensure CUPiD is generalizable and portable to other machines" #92

Closed
wants to merge 2 commits into from

Conversation

shivaniikum
Copy link
Contributor

This pull request addresses issue #89 and contains changes to the config_f.cam6_3_119.FLTHIST_ne30.r328_gamma0.33_soae.001.yaml file at path "examples/nblibrary/config_f.cam6_3_119.FLTHIST_ne30.r328_gamma0.33_soae.001.yaml".

I've removed hardcoded references to glade/ and added global variables to the top of the file and changed to relative file paths throughout the rest of the file. (Note: users should change the start and end year variables to match their cesm output files).

@TeaganKing TeaganKing linked an issue Apr 10, 2024 that may be closed by this pull request
@TeaganKing TeaganKing added the enhancement New feature or request label Apr 10, 2024
@TeaganKing
Copy link
Collaborator

Hi @shivaniikum , thanks for getting this PR started, and nice work finding all of these glade paths.

One main thing that I'm noticing is that we want to generalize how we read in files while also maintaining the functionality of pointing to the current paths.

So, I think it would be useful to set a root directory in a config.yml file parallel to the examples/coupled_model directory (rather than the one in nblibrary), and use this to determine the full paths in config_f.cam6_3_119.FLTHIST_ne30.r328_gamma0.33_soae.001.yaml For instance, the root_directory variable could be something like ../local_cesm_output/ in your config.yml file, and something like /glade/campaign/cesm/development/cross-wg/diagnostic_framework/ in the examples/coupled_model config file.

@mnlevy1981 do you want to add anything else to this suggestion?

@mnlevy1981
Copy link
Collaborator

Nothing to add to your specific suggestion, but I also wanted to ask @shivaniikum to merge the latest main into this branch since it was started before #91 was merged it

@mnlevy1981
Copy link
Collaborator

mnlevy1981 commented Apr 12, 2024

Actually, one thing to add - we're okay with modifying the directory structure under /glade/campaign/cesm/development/cross-wg/diagnostic_framework/, so if setting a root directory in examples/coupled_model/config.yml results in slightly different paths being passed to ADF that's fine. At some point in the future we'll decide on a file structure assuming the root directory is a path generated by CESM, and then we'll make sure any existing examples fit, but until then the directory structure we talked about last week is preferable to what currently exists on the NCAR machine.

@shivaniikum
Copy link
Contributor Author

Thank you for reviewing this PR @mnlevy1981 and @TeaganKing. I will work on making these changes.

@mnlevy1981
Copy link
Collaborator

Two more requests before digging in for an actual review:

  1. It looks like your config.yml file that gets data from examples/local_cesm_output/ hasn't been added to this PR yet (if the decision is to make it examples/local_cesm_output/config.yml, then your root directory could be ./ instead of ../local_cesm_output)
  2. Could you please provide a small README in addition to config.yml describing how to get the necessary data from the web?

@mnlevy1981
Copy link
Collaborator

We'll try to tackle #89 from a fresh PR now that we've made more progress on the key_metrics/ examples

@mnlevy1981 mnlevy1981 closed this Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure CUPiD is generalizable and portable to other machines
3 participants