Skip to content

Remove duplicate callback, add coupler perf pipeline#4504

Merged
nefrathenrici merged 3 commits into
mainfrom
ne/fix
May 21, 2026
Merged

Remove duplicate callback, add coupler perf pipeline#4504
nefrathenrici merged 3 commits into
mainfrom
ne/fix

Conversation

@nefrathenrici
Copy link
Copy Markdown
Member

@nefrathenrici nefrathenrici commented May 20, 2026

This PR removes the duplicate enforce_physical_constraints_callback callback, which was duplicated in common_callbacks and default_model_callbacks by PR #4469.
To catch future performance regressions, this PR also adds an end-to-end coupler performance pipeline. This pipeline runs the flagship amip_progedmf_1m_land_he16 config on clima and checks that the SYPD has not decreased by more than 1%. This pipeline is adapted from the ClimaCore end to end perf pipeline. It is not designed to block PRs, but to track performance regressions in the coupler. It will run on every merged PR and can be manually triggered by adding [perf] to the commit message. This PR increases the SYPD of the pipeline by ~10%, from 0.22 to 0.24 (job).

@nefrathenrici nefrathenrici force-pushed the ne/fix branch 2 times, most recently from e777a9b to cd3262d Compare May 20, 2026 19:12
@nefrathenrici nefrathenrici marked this pull request as ready for review May 20, 2026 19:24
@nefrathenrici nefrathenrici requested a review from szy21 May 20, 2026 19:24
@szy21
Copy link
Copy Markdown
Member

szy21 commented May 20, 2026

It's a bit conerning that this changes MSE for non EDMF runs, as it means a duplicate call of enforce_physical_constraints_callback or set_precomputed_quantites changes the results. @sajjadazimi @trontrytel could you take a look and see if anything is obviously wrong? I'm fine with merging this if we can't figure it out though - we can look at it later.

@szy21
Copy link
Copy Markdown
Member

szy21 commented May 20, 2026

Oh, it changes the MSE of EDMF single column too.

Comment thread .buildkite/coupler_perf/check-sypd.sh Outdated
exit 1
else
echo "✅ SYPD change ($PERCENT_CHANGE%) is okay (threshold: $MIN_PERCENT_CHANGE%)"
fi No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add a line at the end?

Comment thread .buildkite/coupler_perf/pipeline.yml Outdated
slurm_gpus_per_task: 1
slurm_cpus_per_task: 4
slurm_ntasks: 1
slurm_mem: 16GB No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add a line at the end?

Copy link
Copy Markdown
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

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

We talked offline and it's possible that calling cache again changes the results because we have some iterations for cloud fraction computation. I think it's fine.

Copy link
Copy Markdown
Member

@sajjadazimi sajjadazimi left a comment

Choose a reason for hiding this comment

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

Thanks

@nefrathenrici nefrathenrici merged commit 6983c45 into main May 21, 2026
39 checks passed
@nefrathenrici nefrathenrici deleted the ne/fix branch May 21, 2026 15:51
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