Skip to content

Allows construction of AnelasticDynamics and compilation of time_step! with Reactant#463

Draft
dkytezab wants to merge 16 commits into
mainfrom
dkz/issue-build-anelastic
Draft

Allows construction of AnelasticDynamics and compilation of time_step! with Reactant#463
dkytezab wants to merge 16 commits into
mainfrom
dkz/issue-build-anelastic

Conversation

@dkytezab
Copy link
Copy Markdown
Collaborator

See #462.

@giordano
Copy link
Copy Markdown
Member

The history looks messed up 🥲

include("Timesteppers.jl")
using .TimeSteppers

include("MassConservation.jl")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is more correctly call AtmosphereModels (the logic here is that the AtmosphereModels module in the extension will define functions that pertain to src/AtmosphereModels... if that makes sense)

@giordano
Copy link
Copy Markdown
Member

Can we remove all the unrelated commits from this PR? I can do the git surgery if you want.

@dkytezab
Copy link
Copy Markdown
Collaborator Author

that would be perfect, thank you!

@giordano giordano force-pushed the dkz/issue-build-anelastic branch from 557670b to 591f037 Compare February 10, 2026 01:38
@giordano
Copy link
Copy Markdown
Member

Surgery done. Now you need to run

git fetch --all --prune
git checkout dkz/issue-build-anelastic
git reset --hard origin/dkz/issue-build-anelastic

@giordano
Copy link
Copy Markdown
Member

Double check I didn't mess up anything 😃

@dkytezab
Copy link
Copy Markdown
Collaborator Author

LGTM, gonna just add my changes following @glwagner's suggestion

Comment thread ext/BreezeReactantExt/AtmosphereModels.jl Outdated
Comment thread src/AtmosphereModels/set_atmosphere_model.jl Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ext/BreezeReactantExt/AtmosphereModels.jl 0.00% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

dkytezab and others added 2 commits February 9, 2026 17:55
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
Comment thread ext/BreezeReactantExt/AtmosphereModels.jl Outdated
Co-authored-by: Mosè Giordano <765740+giordano@users.noreply.github.com>
@dkytezab
Copy link
Copy Markdown
Collaborator Author

@dkytezab
Copy link
Copy Markdown
Collaborator Author

See CliMA/Oceananigans.jl#5093. @giordano I wonder if we can create separate environments for tests that use Reactant and those that don't? I think we can solve this problem by bumping up the minimum version to 1.11.9 for Reactant tests

@wsmoses
Copy link
Copy Markdown

wsmoses commented Feb 13, 2026

that 1.10 issue is definitely a bug that should be fixed [and from the looks of would not be limited to 1.10, we just got unlucky with julia dispatch on that version]. mwe would be helpful

Comment thread test/runtests.jl Outdated
# support is improved.
if VERSION >= v"1.12"
delete!(testsuite, "reactant_compilation")
delete!(testsuite, "reactant_compilation_compressible")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1.12 should be fine [tho @giordano mentioned slow to compile], was there another reason for removing?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

also any mwes for it being slow are also helpful

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably just bc of the warning that used to be emitted, right? @dkytezab put 1.12 back!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's back!

@glwagner glwagner changed the title Allows construction of AnelasticDynamics and compilation of timestepping Allows construction of AnelasticDynamics and compilation of time_step! with Reactant Feb 19, 2026
@dkytezab dkytezab mentioned this pull request Feb 19, 2026
24 tasks
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.

4 participants