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

Use Broadcast.flatten on master #1782

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

charleskawczynski
Copy link
Member

Inspired by JuliaArrays/StaticArrays.jl#1186, this PR adds the use of Julia master's Broadcast.flatten, which has better inference properties than julia 1.10.

This may negate the need to manually apply the flattening tricks we've needed to in ClimaAtmos (cc @trontrytel), or it may just result in better inference, which seems fine to me, too.

I'm keen to see if any of our existing broken tests due to complexity heuristics start passing.

@Sbozzolo
Copy link
Member

Sbozzolo commented Jun 4, 2024

I don't understand this code, but I think we can trust StaticArrays. Let's see when all the tests pass.

It'd be good to add comments to explain the high-level purpose and intent of this function and when it should be used.

@charleskawczynski
Copy link
Member Author

charleskawczynski commented Jun 4, 2024

I don't understand this code, but I think we can trust StaticArrays. Let's see when all the tests pass.

Yeah, it's a bit complicated, this was just copy-pasted from julia master's Broadcast module so that StaticArrays could benefit from the bleeding edge of julia development in earlier versions of StaticArrays.

It'd be good to add comments to explain the high-level purpose and intent of this function and when it should be used.

Sure, I can add a comment with the julia/StaticArrays PR, and the issues that it fixes for us (one of which we reported)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants