Skip to content

fix: preserve ScalarDiffusivity halo size during conversion - #6093

Merged
glwagner merged 5 commits into
CliMA:mainfrom
RanaPriyansh:fix/5499-scalar-diffusivity-halo
Sep 30, 2026
Merged

glwagner merged 5 commits into
CliMA:mainfrom
RanaPriyansh:fix/5499-scalar-diffusivity-halo

Conversation

@RanaPriyansh

@RanaPriyansh RanaPriyansh commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5499.

Adapting a ScalarDiffusivity closure substitutes the diffusivity type for its required halo size. This can make model construction fail when it compares the halo requirement with an integer.

Read the halo size from the third type parameter in both Adapt.adapt_structure and on_architecture. The regression tests check both conversion paths, coefficient preservation, and the formulation and time discretization.

Validation on dae38053: turbulence_closures/closures passed 343 assertions, memory_allocation passed 47, and unit/quality_assurance passed 107. All three processes exited successfully. These CPU runs used Julia 1.12.7 on macOS arm64. Hosted Linux and GPU results remain separate.

The original baseline checks reproduced both numeric conversion failures. Repaired-source checks covered numeric and callable diffusivities and model halo setup. Those baseline observations belong to the original submission.

Codex used.

@simone-silvestri

Copy link
Copy Markdown
Collaborator

Oh, I thought we already had solved this.

@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.20%. Comparing base (32571e5) to head (be74643).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6093      +/-   ##
==========================================
- Coverage   75.20%   74.20%   -1.01%     
==========================================
  Files         433      435       +2     
  Lines       27195    27692     +497     
==========================================
+ Hits        20453    20549      +96     
- Misses       6742     7143     +401     
Flag Coverage Δ
buildkite 69.07% <100.00%> (+0.01%) ⬆️
distributed_tripolar 20.74% <0.00%> (-0.04%) ⬇️
julia 69.07% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RanaPriyansh

Copy link
Copy Markdown
Contributor Author

Merged current upstream bb0b7a7 into this branch as dae38053, preserving your signature simplification and previous merge history. The PR still changes only the two halo-conversion files.

The old CPU failure came from the inherited predictor-mask path. This update includes upstream's #6101 revert and later #6109 allocation changes. The unchanged allocation selector now passes locally, including the immersed nonhydrostatic case at 128 bytes against a 141-byte limit. The allocation changes come from upstream.

Closures passed 343/343, allocations 47/47, and Aqua 107/107, each with exit zero. Local validation used Julia 1.12.7 on macOS arm64; the failed hosted job used Julia 1.13 on Linux. The new head still needs hosted validation.

Codex used.

@glwagner
glwagner merged commit d834b76 into CliMA:main Sep 30, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adapt.adapt_structure and on_architecture for ScalarDiffusivity swap halo-size with diffusivity-type

3 participants