Skip to content

Add arguments to devbuild.sh script#155

Merged
JulieSchramm merged 3 commits into
ufs-community:developfrom
esmf-org:feature/update_devbuild
Aug 2, 2021
Merged

Add arguments to devbuild.sh script#155
JulieSchramm merged 3 commits into
ufs-community:developfrom
esmf-org:feature/update_devbuild

Conversation

@danrosen25
Copy link
Copy Markdown
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

@danrosen25 danrosen25 force-pushed the feature/update_devbuild branch from b921240 to 0319ce3 Compare July 19, 2021 19:39
@danrosen25 danrosen25 marked this pull request as ready for review July 19, 2021 19:42
Comment thread devbuild.sh Outdated
Comment thread devbuild.sh Outdated
Comment thread devbuild.sh Outdated
Comment thread devbuild.sh Outdated
Comment thread devbuild.sh Outdated
@danrosen25
Copy link
Copy Markdown
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
Copy Markdown
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).

Comment thread devbuild.sh Outdated
@danrosen25 danrosen25 marked this pull request as draft July 20, 2021 19:51
@danrosen25 danrosen25 force-pushed the feature/update_devbuild branch from 0319ce3 to 9247726 Compare July 20, 2021 20:03
@danrosen25 danrosen25 marked this pull request as ready for review July 20, 2021 20:09
@danrosen25 danrosen25 requested a review from gsketefian 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
@danrosen25 danrosen25 force-pushed the feature/update_devbuild branch from 9247726 to e961aad Compare July 20, 2021 20:46
@gsketefian
Copy link
Copy Markdown
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.

Comment thread src/CMakeLists.txt
@JulieSchramm
Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

JulieSchramm commented Jul 26, 2021 via email

@danrosen25
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
Contributor Author

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

@danrosen25 danrosen25 requested a review from mkavulich July 27, 2021 16:17
Copy link
Copy Markdown
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
Copy Markdown

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

Copy link
Copy Markdown

@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