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

Change storage dictionaries to vectors #86

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Conversation

charleskawczynski
Copy link
Member

@charleskawczynski charleskawczynski commented Sep 27, 2024

This PR adds and uses a new function, concrete_dict, in an attempt to improve the type information in the storage, accumulators and counters Dicts.

Closes #85

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.19%. Comparing base (7b08031) to head (2943dd4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   84.90%   85.19%   +0.29%     
==========================================
  Files          13       13              
  Lines         510      520      +10     
==========================================
+ Hits          433      443      +10     
  Misses         77       77              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@charleskawczynski charleskawczynski marked this pull request as ready for review September 28, 2024 01:04
@charleskawczynski charleskawczynski changed the title Improve concreteness of storage Dicts Change storage dictionaries to vectors Sep 28, 2024
src/clima_diagnostics.jl Outdated Show resolved Hide resolved
src/clima_diagnostics.jl Outdated Show resolved Hide resolved
@charleskawczynski charleskawczynski force-pushed the ck/dict_eltypes branch 2 times, most recently from b62eac6 to 2eafeca Compare September 30, 2024 16:08
Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good to me!

Would you like to check that it fixes the issues?

src/clima_diagnostics.jl Show resolved Hide resolved
@charleskawczynski
Copy link
Member Author

I think this looks good to me!

Would you like to check that it fixes the issues?

Yeah, I'll rebase CliMA/ClimaAtmos.jl#3290 once CliMA/ClimaAtmos.jl#3344 lands and try out this branch

@charleskawczynski
Copy link
Member Author

It looks like we get a lot fewer issues with this branch, we can now see the total number of failures! 🎉

https://buildkite.com/clima/climaatmos-ci/builds/20884#019243e1-e861-408d-a36b-6d8c01abc126

Update src/clima_diagnostics.jl

Co-authored-by: Gabriele Bozzola <[email protected]>

Bump patch version
@charleskawczynski charleskawczynski merged commit 1ca7ad5 into main Sep 30, 2024
8 of 9 checks passed
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.

Simplify storage Dict keys
2 participants