Skip to content

Update SurfaceFluxes compat entry (support v0.14) #4109

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update SurfaceFluxes compat entry (support v0.14) #4109

wants to merge 2 commits into from

Conversation

@akshaysridhar
Copy link
Member

@akshaysridhar akshaysridhar commented Nov 18, 2025 •
edited
Loading

No description provided.

@akshaysridhar akshaysridhar force-pushed the as/up-sfcompat branch 5 times, most recently from 0afaea6 to 836d1de Compare November 18, 2025 17:00
juliasloan25
juliasloan25 reviewed Nov 18, 2025
View reviewed changes
Copy link
Member

@juliasloan25 juliasloan25 left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but what is new in SurfaceFluxes v0.14? I see that it can support non-constant roughness lengths now, but it isn't clear to me how one would set that up.

Also, do you know why the ref counter has to change?

@szy21
Copy link
Member

szy21 commented Nov 18, 2025

Also this didn't change SurfaceFluxes in the Manifest - why does it have behavior change?

@akshaysridhar
Copy link
Member Author

akshaysridhar commented Nov 18, 2025

Also this didn't change SurfaceFluxes in the Manifest - why does it have behavior change?

I'll double check; I assumed we were not checking in the Manifest.toml at the top-level (which shows, locally, that I'm tracking [49b00bb7] SurfaceFluxes v0.14.0).

@akshaysridhar
Copy link
Member Author

akshaysridhar commented Nov 18, 2025

This looks fine to me, but what is new in SurfaceFluxes v0.14? I see that it can support non-constant roughness lengths now, but it isn't clear to me how one would set that up.

Also, do you know why the ref counter has to change?

There is a breaking change to the interfaces in the AbstractSurfaceConditions in SF ; so the current method in CA requires the kwarg separator for the default value to be applied correctly. There are also changes to the solver / iterator structure in v0.14.0 within SF.

@akshaysridhar
modified: Project.toml
9f6d4f3 
      + semicolon separator for kwargs
	modified:   src/surface_conditions/surface_conditions.jl
	modified:   test/coupler_compatibility.jl
	modified:   reproducibility_tests/ref_counter.jl

        Check-in updated Manifest for buildkite env
	modified:   .buildkite/Manifest-v1.11.toml
@akshaysridhar
modified: .buildkite/Manifest-v1.11.toml
a612452 
	modified:   reproducibility_tests/ref_counter.jl
@juliasloan25
Copy link
Member

juliasloan25 commented Nov 18, 2025

This looks fine to me, but what is new in SurfaceFluxes v0.14? I see that it can support non-constant roughness lengths now, but it isn't clear to me how one would set that up.
Also, do you know why the ref counter has to change?

There is a breaking change to the interfaces in the AbstractSurfaceConditions in SF ; so the current method in CA requires the kwarg separator for the default value to be applied correctly. There are also changes to the solver / iterator structure in v0.14.0 within SF.

Okay, that makes sense. But if it's only an interface change and restructuring, why did the results change?

@szy21
Copy link
Member

szy21 commented Nov 19, 2025

The allocation increase is quite substantial - do you have any idea why?

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

Reviewers

@juliasloan25 juliasloan25 juliasloan25 left review comments

@szy21 szy21 Awaiting requested review from szy21

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@akshaysridhar @szy21 @juliasloan25