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 arguments to devbuild.sh script #155

Merged

Conversation

danrosen25
Copy link
Contributor

@danrosen25 danrosen25 commented Jul 19, 2021

DESCRIPTION OF CHANGES:

  • devbuild.sh documented with --help
  • automatically determines compiler based on machine name
  • added options for app, ccpp suite, enable options, disable options
  • added option for build type DEBUG, RELEASE, RELWITHDEBINFO
  • added option for build jobs, default 4
  • added verbosity option for make

TESTS CONDUCTED:

Tested each devbuild.sh option in isolation. Tested on Jet (needs testing and confirmation of platform/compiler defaults on other machines)

DEPENDENCIES:

None

DOCUMENTATION:

Usage: ./devbuild.sh PLATFORM [OPTIONS]...

PLATFORM
      name of machine you are building on
      (e.g. cheyenne | hera | jet | orion | wcoss)

OPTIONS
  -h, --help
      show this help guide
  --compiler=COMPILER
      compiler to use; default depends on platform
      (e.g. intel | gnu | cray | gccgfortran)
  --app=APPLICATION
      weather model application to build
      (e.g. ATM | ATMW | S2S | S2SW)
  --ccpp="CCPP_SUITE1,CCPP_SUITE2..."
      CCCP suites to include in build; delimited with ','
  --enable-options="OPTION1,OPTION2,..."
      enable ufs-weather-model options; delimited with ','
      (e.g. 32BIT | INLINE_POST | UFS_GOCART | MOM6 | CICE6 | WW3 | CMEPS)
  --disable-options="OPTION1,OPTION2,..."
      disable ufs-weather-model options; delimited with ','
      (e.g. 32BIT | INLINE_POST | UFS_GOCART | MOM6 | CICE6 | WW3 | CMEPS)
  --continue
      continue with existing build
  --clean
      removes existing build; overrides --continue
  --build-dir=BUILD_DIR
      build directory
  --install-dir=INSTALL_DIR
      installation prefix
  --build-type=BUILD_TYPE
      build type; defaults to RELEASE
      (e.g. DEBUG | RELEASE | RELWITHDEBINFO)
  --build-jobs=BUILD_JOBS
      number of build jobs; defaults to 4
  -v, --verbose
      build with verbose output

NOTE: This script is for internal developer use only;
See User's Guide for detailed build instructions

CONTRIBUTORS:

@danrosen25

devbuild.sh Outdated Show resolved Hide resolved
devbuild.sh Outdated Show resolved Hide resolved
devbuild.sh Outdated Show resolved Hide resolved
devbuild.sh Outdated Show resolved Hide resolved
devbuild.sh Outdated Show resolved Hide resolved
@danrosen25
Copy link
Contributor Author

@gsketefian I was looking at the ufs-weather-model CMakeList.txt and they use -DDEBUG to set CMAKE_BUILD_TYPE. Do you want me to replace --build-type=? with --debug?
https://github.com/ufs-community/ufs-weather-model/blob/develop/CMakeLists.txt#L95

@gsketefian
Copy link
Collaborator

@gsketefian I was looking at the ufs-weather-model CMakeList.txt and they use -DDEBUG to set CMAKE_BUILD_TYPE. Do you want me to replace --build-type=? with --debug?
https://github.com/ufs-community/ufs-weather-model/blob/develop/CMakeLists.txt#L95

I like --build-type because --debug sounds to me like a true/false flag (but isn't).

devbuild.sh Outdated Show resolved Hide resolved
@danrosen25 danrosen25 marked this pull request as draft July 20, 2021 19:51
@danrosen25 danrosen25 marked this pull request as ready for review July 20, 2021 20:09
@danrosen25 danrosen25 marked this pull request as draft July 20, 2021 20:32
@danrosen25 danrosen25 marked this pull request as ready for review July 20, 2021 20:36
* Documented with --help
* Automatically determines machine name
* Automatically determines compiler based on machine name
* Added options for app, ccpp suite, and components
* Added option for build type DEBUG, RELEASE, RELWITHDEBINFO
* Added option for build jobs, default 4
* Added verbosity option for make
@gsketefian
Copy link
Collaborator

@danrosen25 This looks good now. I guess it should be tested on one more machine, e.g. Hera. I would volunteer but have some other things to finish. I can get to it say Thursday, but if someone else can test it beforehand, that would be great.

@JulieSchramm
Copy link

@danrosen25 I can test this on Hera and Cheyenne. I am out Friday, and Cheyenne is down next week, so I will start with Hera. Let me know when it is ready for testing.

@danrosen25
Copy link
Contributor Author

danrosen25 commented Jul 22, 2021

@JeffBeck-NOAA @JulieSchramm @mkavulich @gsketefian
The only question I have right now is
Do you want me to change --component=<COMPONENT1,COMPONENT2,...> to --enable-option=<OPTION1,OPTION2,...> and --disable-option=<OPTION1,OPTION2,...>, which better represents enabling and disabling options in https://github.com/ufs-community/ufs-weather-model/blob/develop/CMakeLists.txt. This can be used to enable/disable any option for the ufs-weather-model including 32BIT and INLINE_POST

@JulieSchramm
Copy link

When I run ./devbuild.sh hera --build-dir=build_intel --install-dir=../bin_intel, the build goes in the build directory and the executables are installed in the bin directory. Is this the expected behavior?

@JulieSchramm
Copy link

Oops, need the --platform. I'm not a big fan of the machine-setup.sh script. It figures out the machine name from the available disk names. This caused problems when "yellowstone" became "cheyenne" and the disk names changed, and this had to be fixed in the many instances of this script. Can the platform be a required argument, and then we wouldn't need machine-setup.sh?

@danrosen25
Copy link
Contributor Author

@JulieSchramm
I removed the platform requirement. ./devbuild.sh hera will throw an error because the argument is unknown. ./devbuild.sh will automatically determine that you're running on hera or ./devbuild.sh --platform=hera will manually specify hera.

--build-dir=build_intel will build into <CURRENT_DIRECTORY>/build_intel
and --install-dir=../bin_intel sets CMAKE_INSTALL_PREFIX=../bin_intel, which will install binaries into <CURRENT_DIRECTORY>/build_intel/../bin_intel/bin, libraries into <CURRENT_DIRECTORY>/build_intel/../bin_intel/lib, share into <CURRENT_DIRECTORY>/build_intel/../bin_intel/share, and include into <CURRENT_DIRECTORY>/build_intel/../bin_intel/include
Is that what you want?

@JulieSchramm
Copy link

JulieSchramm commented Jul 26, 2021 via email

@danrosen25
Copy link
Contributor Author

@JulieSchramm
Oh, I see that I'm just ignoring arguments that don't start with -, so yes you can type ./devbuild.sh hera but hera is ignored. Do you want me to make PLATFORM a required argument since the machine_setup.sh script can be buggy? I would change usage to Usage: .devbuild.sh PLATFORM [OPTIONS]... and remove --platform=PLATFORM.

@JulieSchramm
Copy link

JulieSchramm commented Jul 27, 2021 via email

@danrosen25 danrosen25 marked this pull request as draft July 27, 2021 15:08
* remove --components
* add --enable-options
* add --disable-options
@danrosen25 danrosen25 marked this pull request as ready for review July 27, 2021 16:08
@danrosen25
Copy link
Contributor Author

@JulieSchramm I made PLATFORM a required argument.
@mkavulich I changed --components to --enable-options and --disable-options

Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

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

Looks good, my concerns have been addressed. Once @JulieSchramm is okay with the updates I believe this PR is good to go.

@JulieSchramm
Copy link

I will test on Cheyenne next week Aug 2 or 3 when the computer is back up.

Copy link

@JulieSchramm JulieSchramm left a comment

Choose a reason for hiding this comment

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

Tested on Jet, Hera and Cheyenne.

@JulieSchramm JulieSchramm merged commit 5ab9a08 into ufs-community:develop Aug 2, 2021
@danrosen25 danrosen25 deleted the feature/update_devbuild branch August 2, 2021 16:26
christinaholtNOAA pushed a commit to christinaholtNOAA/ufs-srweather-app that referenced this pull request May 23, 2022
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.

4 participants