Skip to content

Conversation

@petebachant
Copy link
Member

@petebachant petebachant commented Oct 14, 2025

This uses CliMA/ClimaCore.jl#2376 to provide more useful CUDA kernel names in benchmarks.

TODO

  • Can we format the kernel names with anything other than underscores as separators? --> No, all non-alphanumeric characters get converted to underscores.
  • Do we want to do this on the benchmarks that use benchmark.jl, not benchmark_step.jl? --> we could, but probably not useful for CI yet.
  • Update Buildkite pipeline to use this feature
  • Switch to non-dev ClimaCore

@petebachant petebachant marked this pull request as draft October 14, 2025 16:58

- group: "Reproducibility infrastructure"
steps:

Copy link
Member Author

Choose a reason for hiding this comment

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

These changes were made by a YAML auto-formatter in VS Code. Is there a style guide I might be breaking here?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure... this is something I have been wondering as well. I considered following this example, which is used in Buildkite's docs.

@petebachant petebachant marked this pull request as ready for review October 22, 2025 18:30
@petebachant
Copy link
Member Author

@dennisYatunin @imreddyTeja thoughts on the timeouts here? Should I increase the limit or disable kernel renaming?

@imreddyTeja
Copy link
Member

@dennisYatunin @imreddyTeja thoughts on the timeouts here? Should I increase the limit or disable kernel renaming?

What is the advantage of using kernel renaming in climaatmos-ci for buildkite steps that don't profile? Is the idea to run the profiler at the end of each simulation?

Comparing the buildkite for this PR to the main's last buildkite run shows a ~40% slowdown, but everything after the first step doesn't seem to be affected. I'm not sure if that cost is worth it at the moment.

@petebachant
Copy link
Member Author

What is the advantage of using kernel renaming in climaatmos-ci for buildkite steps that don't profile? Is the idea to run the profiler at the end of each simulation?

I believe that was Dennis's vision, and then we'd process and summarize all of the profiling results together in a later step.

@petebachant
Copy link
Member Author

Alright, so now that CliMA/ClimaCore.jl#2376 is merged, do I need to make a ClimaCore release, or should I simply update the .buildkite project to refer to a specific ClimaCore Git commit?

I suppose I could revert any Buildkite changes here and just keep the updates to the benchmarking script.

- group: "Benchmarks"
steps:

- label: ":computer: Benchmark: CPU baroclinic wave moist"
Copy link
Member

Choose a reason for hiding this comment

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

I think this label is incorrect

redirect_stderr(IOContext(stderr, :stacktrace_types_limited => Ref(false)))
import ClimaComms
ClimaComms.@import_required_backends
import ClimaCore
Copy link
Member

Choose a reason for hiding this comment

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

Is this import used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. It's no longer used. ClimaComms was also imported twice. We really need to figure out how to get linting working!

@imreddyTeja
Copy link
Member

Alright, so now that CliMA/ClimaCore.jl#2376 is merged, do I need to make a ClimaCore release, or should I simply update the .buildkite project to refer to a specific ClimaCore Git commit?

I suppose I could revert any Buildkite changes here and just keep the updates to the benchmarking script.

I think we should make a ClimaCore release. It would be nice if we updated the prettytables compat before releasing.
After the release and updating the manifest, I think this PR is good to go.

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.

3 participants