-
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
Add flamegraphs #236
Add flamegraphs #236
Conversation
e2173ea
to
9cb0541
Compare
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!
.buildkite/pipeline.yml
Outdated
artifact_paths: "experiments/AMIP/moist_mpi_earth/output/amip/target_amip_n32_shortrun_artifacts/*" | ||
env: | ||
CLIMACORE_DISTRIBUTED: "MPI" | ||
agents: | ||
slurm_ntasks: 32 | ||
slurm_mem_per_cpu: 16G | ||
|
||
# performance tests | ||
- label: ":rocket: performance: flame graph and allocation tests" |
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.
it might be helpful each of the performance tests has a unique label
and artifact_paths
so we can tell them apart more easily and avoid overwriting outputs
@@ -320,7 +324,7 @@ cs = CoupledSimulation{FT}( | |||
coupler_fields, | |||
parsed_args, | |||
conservation_checks, | |||
tspan, | |||
[tspan[1], tspan[2]], |
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.
would it be simpler to initialize tspan
as an array of length 2 instead? i.e. tspan = [Int(0), t_end]
, then we can just pass in tspan
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.
Doing it this way allows specifying tspan as a Tuple (like in Atmos's global arguments which overwrites our tspan) or as an array. I know... and I agree that we should unify this, once we don't have synonymous global variables lingering about in the background. :)
transfer + modify from ClimaCore/ClimaAtmos add alloc benchmarks add buildkite driver rev allocs docs activate docs clean fix doc fix doc fix check in perf Manifest bkite flag order fix scope fix FT redef error fix fix stash fix clean rebase fixes rebase fix bkite rev1
06f719b
to
3346bf9
Compare
bors r+ |
Purpose
Addresses the first component of #229
To-do