Skip to content

Comments

Update configs and leaderboard#1753

Open
ph-kev wants to merge 2 commits intomainfrom
kp/update-config
Open

Update configs and leaderboard#1753
ph-kev wants to merge 2 commits intomainfrom
kp/update-config

Conversation

@ph-kev
Copy link
Member

@ph-kev ph-kev commented Feb 23, 2026

This PR updates the leaderboard for diagnostics in pressure coordinates by throwing a warning if no diagnostics in pressure coordinates are found. Furthermore, this PR adds diagnostics in pressure coordinates for all configs in amip_configs, longrun_configs, and nightly_configs.

@ph-kev ph-kev requested review from juliasloan25 and szy21 February 23, 2026 22:52
Copy link
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.

Thanks! Would it be easier if we make them default atmos diagnostics?

@ph-kev
Copy link
Member Author

ph-kev commented Feb 23, 2026

Thanks! Would it be easier if we make them default atmos diagnostics?

I am not too sure if they should be part of the default since outputting diagnostics in pressure coordinates is computationally expensive.

@szy21
Copy link
Member

szy21 commented Feb 23, 2026

Thanks! Would it be easier if we make them default atmos diagnostics?

I am not too sure if they should be part of the default since outputting diagnostics in pressure coordinates is computationally expensive.

Sounds good. How expensive is it?

@ph-kev
Copy link
Member Author

ph-kev commented Feb 24, 2026

Thanks! Would it be easier if we make them default atmos diagnostics?

I am not too sure if they should be part of the default since outputting diagnostics in pressure coordinates is computationally expensive.

Sounds good. How expensive is it?

Each accumulation / compute step of the diagnostics involves a kernel call to interpolation data from z coordinates to pressure coordinates. This isn't too expensive since compute_every is 6 hours, but it can be expensive if compute step is called more often. I haven't benchmark this, so it might be faster than what I think it is. It is probably okay to include them as part of the default atmos diagnostics.

@szy21
Copy link
Member

szy21 commented Feb 24, 2026

Thanks! Would it be easier if we make them default atmos diagnostics?

I am not too sure if they should be part of the default since outputting diagnostics in pressure coordinates is computationally expensive.

Sounds good. How expensive is it?

Each accumulation / compute step of the diagnostics involves a kernel call to interpolation data from z coordinates to pressure coordinates. This isn't too expensive since compute_every is 6 hours, but it can be expensive if compute step is called more often. I haven't benchmark this, so it might be faster than what I think it is. It is probably okay to include them as part of the default atmos diagnostics.

Feel free to merge this PR, and we can add them to default diagnostics later if it is not too expensive.

Copy link
Member

@juliasloan25 juliasloan25 left a comment

Choose a reason for hiding this comment

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

This looks fine to me but I am a little worried about the diagnostics duplicated in so many files leading to problems later on e.g. if we don't update all of them. If we don't want to put them in the default atmosphere config, maybe we can add one like clima_diagedmf.yml but including the diagnostics - clima_diagedmf_longrun.yml or something like that

@szy21
Copy link
Member

szy21 commented Feb 24, 2026

This looks fine to me but I am a little worried about the diagnostics duplicated in so many files leading to problems later on e.g. if we don't update all of them. If we don't want to put them in the default atmosphere config, maybe we can add one like clima_diagedmf.yml but including the diagnostics - clima_diagedmf_longrun.yml or something like that

Oh, we can just add them in config/atmos_configs/, if it doesn't make ci slower? (not sure if period: 1months and compute_every: 6hours will cause a problem for short ci runs though)

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