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

remove G2G arg from run_command, consolidate gpu logic in buildenv #76

Merged
merged 3 commits into from
Dec 17, 2021

Conversation

mcgibbon
Copy link
Collaborator

@mcgibbon mcgibbon commented Dec 17, 2021

Purpose

run_command currently takes a G2G argument we have hard-coded to always be false. This PR removes that argument, and moves several environment variables from pace into buildenv.

Depends on ai2cm/buildenv#32.

Infrastructure changes:

  • G2G is removed as an argument for run_command
  • DOCKER_BUILDKIT=1 is set in buildenv, not pace
  • PYTHONOPTIMIZE=TRUE is set in run_on_daint.sh, not in the slurm script
  • updated pace-util jenkins.sh to match buildenv run_command api

Checklist

Before submitting this PR, please make sure:

  • You have followed the coding standards guidelines established at Code Review Checklist.
  • Docstrings and type hints are added to new and updated routines, as appropriate
  • All relevant documentation has been updated or added (e.g. README, CONTRIBUTING docs)
  • Unit tests are added or updated for non-stencil code changes

Additionally, if this PR contains code authored by new contributors:

  • The names of all the new contributors have been added to CONTRIBUTORS.md

@mcgibbon
Copy link
Collaborator Author

Performance test launched https://jenkins.ginko.ch/job/pace-fv3core-performance-test/91/.

@mcgibbon
Copy link
Collaborator Author

Submitted performance test again for gpu, will appear at https://jenkins.ginko.ch/job/pace-fv3core-performance-test/93/.

Copy link
Contributor

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

LGTM

@mcgibbon mcgibbon marked this pull request as ready for review December 17, 2021 20:27
@mcgibbon
Copy link
Collaborator Author

Because the buildenv changes are backwards incompatible I will merge to main pointing to the buildenv branch, then merge the branch on buildenv, then merge a dependabot PR.

@mcgibbon mcgibbon enabled auto-merge (squash) December 17, 2021 20:29
@mcgibbon mcgibbon merged commit 9cda491 into main Dec 17, 2021
@mcgibbon mcgibbon deleted the feature/move_g2g_to_buildenv branch December 17, 2021 23:10
mcgibbon added a commit to ai2cm/buildenv that referenced this pull request Dec 17, 2021
## Infrastructure changes:

- G2G is removed as an argument for run_command
- DOCKER_BUILDKIT=1 is set in buildenv, not pace
- PYTHONOPTIMIZE=TRUE is set in run_on_daint.sh, not in the slurm script
- updated pace-util jenkins.sh to match buildenv run_command api

Linked to ai2cm/pace#76
@mcgibbon mcgibbon mentioned this pull request Jan 4, 2022
4 tasks
mcgibbon added a commit that referenced this pull request Jan 4, 2022
## Purpose

A bug was introduced by #76 where an argument was removed but the argument number for the final argument (script filename) was not decremented in buildenv. This PR fixes that bug.

Depends on ai2cm/buildenv#35.

## Infrastructure changes:

- Fixed bug which causes scheduler script argument to be ignored in buildenv's run_command
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