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

Make Nh a dynamic parameter #2005

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Make Nh a dynamic parameter #2005

merged 1 commit into from
Oct 1, 2024

Conversation

charleskawczynski
Copy link
Member

This PR reverts #1894 as we are running into extremely large compile times.

Please note that this may cause runtime slowdowns, because Nh will be (again) no longer statically known.

Perhaps we can revert this PR once JuliaLang/julia#55807 is resolved.

@charleskawczynski
Copy link
Member Author

I'm going to test this out in Atmos, first.

@charleskawczynski
Copy link
Member Author

Based on https://buildkite.com/clima/climacoupler-amip/builds/86#01924641-c4af-4d7f-aae0-b705fd71593b and https://buildkite.com/clima/climacoupler-amip/builds/81#01924583-5e69-4cc3-b445-76bc95b26646, there's about a ~2% slowdown. That said, I know that compilation time in a handful of Atmos CI jobs are significantly improved with this PR. So let's merge for now, and then we can revert it down the road when some of the julia latency issues are alleviated.

@charleskawczynski charleskawczynski merged commit c6bfad8 into main Oct 1, 2024
21 of 23 checks passed
@charleskawczynski charleskawczynski deleted the ck/dynamic_Nh branch October 1, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants