Skip to content
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 different sizes for dust and sea salt #564

Merged
merged 1 commit into from
Feb 5, 2025
Merged

add different sizes for dust and sea salt #564

merged 1 commit into from
Feb 5, 2025

Conversation

szy21
Copy link
Member

@szy21 szy21 commented Jan 29, 2025

Purpose

co-authored-by: @charleskawczynski

To-do

Content


  • I have read and checked the items on the review checklist.

@szy21 szy21 marked this pull request as ready for review January 30, 2025 18:00
@szy21
Copy link
Member Author

szy21 commented Jan 30, 2025

This works fine in amip: https://buildkite.com/clima/climacoupler-coarse-nightly-amip/builds/237. It doesn't change the result right now. I need to update ClimaAtmos to see if this new capability improves the simulation. I would like to make a patch release after this is merged.

@szy21
Copy link
Member Author

szy21 commented Jan 30, 2025

Actually this is going to be a breaking change as we add more aerosols in RRTMGP, and we need to initialize them correctly in ClimaAtmos. I'll make a patch release with the current main.

@sriharshakandala
Copy link
Member

sriharshakandala commented Jan 31, 2025

Does this not work for our purpose to use (compute) one statistically representative particle size for each particle type, statistically computed based on available particle size distribution for that particle type, and use that instead?

@szy21
Copy link
Member Author

szy21 commented Jan 31, 2025

We talked about it offline. This gives us the most accurate optics with information about different size bins, but is slow. I can think about if we can come up with effective radius for dust and sea salt when we have an aerosol model.

@sriharshakandala
Copy link
Member

I started a benchmarking build here: https://buildkite.com/clima/rrtmgp-clima-a100-pipeline/builds/45

@szy21
Copy link
Member Author

szy21 commented Jan 31, 2025

I started a benchmarking build here: https://buildkite.com/clima/rrtmgp-clima-a100-pipeline/builds/45

I don't think the benchmark is accurate as it reads the aerosol state from the rrtmgp example. So no matter how many aerosol categories are added, only 2 type (1 and 3) are used.

@sriharshakandala
Copy link
Member

We talked about it offline. This gives us the most accurate optics with information about different size bins, but is slow. I can think about if we can come up with effective radius for dust and sea salt when we have an aerosol model.

I am ok with this if this is a

I started a benchmarking build here: https://buildkite.com/clima/rrtmgp-clima-a100-pipeline/builds/45

I don't think the benchmark is accurate as it reads the aerosol state from the rrtmgp example. So no matter how many aerosol categories are added, only 2 type (1 and 3) are used.

Yes. This only checks if there are no performance regressions from this PR!

@sriharshakandala
Copy link
Member

sriharshakandala commented Jan 31, 2025

We talked about it offline. This gives us the most accurate optics with information about different size bins, but is slow. I can think about if we can come up with effective radius for dust and sea salt when we have an aerosol model.

As discussed, this use case deviates from the design intent, which expects one representative particle size for each particle type.

As discussed, we are ok with this as a temporary fix, if immediately needed.

Eventually, we should either try to get one statistically representative particle size for each particle type, roll this back, or we can redesign this so that users can choose number of particle types they would like to use and number of bins for each particle type!

@szy21
Copy link
Member Author

szy21 commented Jan 31, 2025

Sounds good, thanks @sriharshakandala.

@szy21 szy21 force-pushed the zs/ck/aerosol branch 2 times, most recently from cde4213 to 670eddb Compare February 4, 2025 06:10
@szy21 szy21 closed this Feb 5, 2025
@szy21 szy21 reopened this Feb 5, 2025
@szy21 szy21 closed this Feb 5, 2025
@szy21 szy21 reopened this Feb 5, 2025
@szy21 szy21 enabled auto-merge February 5, 2025 21:39
@szy21 szy21 merged commit 9dd6755 into main Feb 5, 2025
9 of 11 checks passed
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.

2 participants