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

Add upp as a submodule #21

Merged
merged 5 commits into from
May 10, 2024
Merged

Add upp as a submodule #21

merged 5 commits into from
May 10, 2024

Conversation

aerorahul
Copy link
Contributor

@aerorahul aerorahul commented May 9, 2024

This PR:

  • adds UPP as a submodule.
  • adds a script to build standalone UPP with GTG
  • moves build scripts from ush to sorc as NCO would like it to be
  • puts executables in the EE2 vertical structure
  • updates README to clone recursively and build all (upp and wafs executables)

The checkout and build was tested on WCOSS2.

@YaliMao-NOAA
Copy link
Collaborator

YaliMao-NOAA commented May 10, 2024

@aerorahul Thank you very much for the great help. I have a few questions.

  1. Does it look good to mix build scripts and wafs run scripts in the same ush folder? Any suggestion to differentiate them?
  2. I still think it's a good idea to separate a checkout (or install) script from the build script. Here are my reasons.
  • Though I can clone upp code using 'recursive' option at the very first step, gtg code can not be cloned. gtg code is cloned by the build_upp script. If one checkout (or install) script is created, we can clone upp code then gtg code step by step in one script.
  • In the build_upp script, we don't need to clone gtg code every time. When either UPP or GTG code is modified, build_upp should just compile the code, not involve cloning code.
  1. If you agree with my proposal of a separate checkout script, in readme, remove the recursive clone line and add this checkout script instead.

I can do option 2 as the above on my own after I merge your PR.

@aerorahul
Copy link
Contributor Author

@aerorahul Thank you very much for the great help. I have a couple questions.

  1. Does it look good to mix build scripts and wafs run scripts in the same ush folder? Any suggestion to differentiate them?

There are no build scripts in ush/ detect_machine.sh and module-setup.sh are acceptable in ush/.

  1. I still think it's a good idea to separate checkout script from build script. Here are my reasons.
  • Though I can clone upp to wafs using 'recursive' option at the very first step, gtg code can not be cloned. From build_upp, I see gtg code is checked out. If one checkout script is created, we can simply clone the inventory, cd the folder, run the checkout script which clones upp code then gtg code step by step.
  • In the build_upp script, we don't need to clone gtg code every time. When either UPP or GTG code is modified, build_upp should just compile the code, not involve cloning code.

It is up to you to on how you wish to set this up.

  1. If you agree with my proposal of a separate checkout script, in readme, remove the recursive clone line and add this checkout script instead.

I can do option 2 as the above on my own after I merge your PR.

Sounds good!

@YaliMao-NOAA
Copy link
Collaborator

YaliMao-NOAA commented May 10, 2024

I mean both detect_machine.sh and module-setup.sh are not related to the actual WAFS product run.

There are no build scripts in ush/ detect_machine.sh and module-setup.sh are acceptable in ush/.

@aerorahul
Copy link
Contributor Author

I mean both detect_machine.sh and module-setup.sh are not related to the actual WAFS product run.

There are no build scripts in ush/ detect_machine.sh and module-setup.sh are acceptable in ush/.

They are scripts used in the WAFS application to build and are acceptable in ush/. If you remove them, then feel free to adjust as necessary.

@YaliMao-NOAA YaliMao-NOAA merged commit f1c6387 into develop May 10, 2024
@aerorahul aerorahul deleted the feature/clone-build-link branch May 24, 2024 12:56
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.

2 participants