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 UniversalSize to spaces #1997

Closed
wants to merge 1 commit into from

Conversation

charleskawczynski
Copy link
Member

It seems that compile times are due to Nh. I would argue that adding a type parameter that has a single value for an entire simulation shouldn't be an issue. However, in the light of JuliaLang/julia#55807, it's possible that this isn't the case. I'd like to make progress on alleviating long compile times--I've observed that a job in ClimaAtmos took about 26 minutes to complete the first timestep, and each subsequent step is on the order of, or less than, one second.

This PR adds universal sizes to the spaces. The hope is that we can preserve the performance benefit of knowing static array dimensions (#11) for most broadcast expressions, while reducing compile times. Here are some trade-offs with this PR:

  • We can remove Nh (and Nv) from the type domain, and instead grab them from the space for broadcast operations for field-style expressions
  • For DataLayout broadcast expressions we won't have a space to grab the universal size (containing Nh, Nv) from, so those kernels will revert to using runtime Nh/Nv.

Unfortunately, this means that diagnostic edmf, which performs lost of level operations on DataLayouts, will fall back to a somewhat slower kernel (as it was before Nh/Nv was static).

@charleskawczynski
Copy link
Member Author

I've come to realize, that this is really just a patch solution, and the main issue is that we don't have access to spaces in broadcast expressions involving only datalayouts, of which there are many in diagnostic edmf. So, while this does offer an appealing trade-off, the real problem is that compilation is so expensive when Nh is in the type space, which I'm hoping gets fixed in new versions of Julia.

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.

1 participant