Skip to content

Use Broadcast.flatten on master - #1782

Closed
charleskawczynski wants to merge 1 commit into
mainfrom
ck/bcflatten
Closed

Use Broadcast.flatten on master#1782
charleskawczynski wants to merge 1 commit into
mainfrom
ck/bcflatten

Conversation

@charleskawczynski

Copy link
Copy Markdown
Contributor

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

Sbozzolo commented Jun 4, 2024

Copy link
Copy Markdown
Contributor

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

charleskawczynski commented Jun 4, 2024

Copy link
Copy Markdown
Contributor Author

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)

@charleskawczynski

Copy link
Copy Markdown
Contributor Author

I don't think we need this anymore, now that atmos has updated to Julia 1.11.

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