-
Notifications
You must be signed in to change notification settings - Fork 5
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
unify buildkite pipelines #443
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but might be a good idea to try running it
see longrun here: https://buildkite.com/clima/climacoupler-longruns/builds/212 |
.buildkite/longruns/pipeline.yml
Outdated
@@ -1,22 +1,19 @@ | |||
agents: | |||
queue: central | |||
slurm_mem: 8G |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're hitting CliMA/slurm-buildkite#47
This is applied to all jobs, but some other jobs also specify slurm_mem_per_cpu
, which causes conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh okay, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it may be similarly problematic to specify both slurm_ntasks
and slurm_tasks_per_node
? e.g. here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that can do weird things.
@LenkaNovak it looks like these changes will fix the longrun issues we had that prevented the coupler reports from being generated :) |
f5b8fba
to
f7ee3da
Compare
Awesome! Thanks for pursuing this @juliasloan25 and @simonbyrne for your help! Once this is merged we can finalize #456 and start profiling the DYAMOND run with @sriharshakandala 👏 |
Maybe just a clarification, @simonbyrne , wouldn't removing |
Looking at the docs:
What I would generally suggest is using either just |
I can it change to Also, as of this PR our target AMIP longrun fails due to numerical instability. It's hard to tell which PR introduced this, since the last time the longruns successfully ran was Sept 17, and between then and now the target AMIP run was failing due to HDF5 errors. It could have been the updates to ClimaAtmos v0.16.0 or v0.16.1. |
Sounds good. Thanks for the pointer! @juliasloan25 Would it be possible to do this, i.e. specify |
f7ee3da
to
35b0763
Compare
35b0763
to
4cce0a6
Compare
bors r+ |
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Purpose
remove some environment variables from our buildkite pipelines, and unify standard and longrun pipelines
closes #441