Skip to content

Simpler horizontal remapping methods for diagnostics #2455

Merged
akshaysridhar merged 1 commit intomainfrom
as/bilinear-horizontal-interp
Feb 20, 2026
Merged

Simpler horizontal remapping methods for diagnostics #2455
akshaysridhar merged 1 commit intomainfrom
as/bilinear-horizontal-interp

Conversation

@akshaysridhar
Copy link
Member

@akshaysridhar akshaysridhar commented Feb 4, 2026

Enables simpler bilinear interpolation option (see ClimaInterpolations.jl) within the Remapping module.

Example:
Screenshot 2026-02-04 at 1 16 01 PM

@akshaysridhar
Copy link
Member Author

Nq = 2 ; e.g.
Screenshot 2026-02-06 at 4 14 01 PM

@akshaysridhar akshaysridhar force-pushed the as/bilinear-horizontal-interp branch 4 times, most recently from 2bbd332 to 5b876f1 Compare February 9, 2026 17:57
@akshaysridhar akshaysridhar marked this pull request as ready for review February 9, 2026 20:52
Copy link
Member

@ph-kev ph-kev 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 the tests in ClimaDiagnostics are failing because a default wasn't provided for horizontal_method in the Remapper object?

I think someone who know more about the math of interpolation should look at this too.

@akshaysridhar
Copy link
Member Author

I think the tests in ClimaDiagnostics are failing because a default wasn't provided for horizontal_method in the Remapper object?

I think someone who know more about the math of interpolation should look at this too.

Thanks @ph-kev : API has been updated to fix downstream failures - and struct properties updated in BilinearRemapping to maintain the current API (+ minor docs fixes following your suggestions)

@akshaysridhar akshaysridhar changed the title Try simpler horizontal remapping methods for diagnostics Simpler horizontal remapping methods for diagnostics Feb 10, 2026
@juliasloan25 juliasloan25 self-requested a review February 12, 2026 23:55
@akshaysridhar akshaysridhar force-pushed the as/bilinear-horizontal-interp branch from 8f572ee to 806b84a Compare February 17, 2026 22:52
@akshaysridhar
Copy link
Member Author

Adding some context from ClimaAtmos runs via @oalcabes :
The default remapping method result in the following outcome for some representative diagnostic variable (cloud top height)

julia> cltz.data |> extrema
(-2279.2927f0, 13606.046f0)`

wherein the -2.2km minimum is clearly unphysical, compared with the simpler bilinear remapping (both calculations on identical target meshes) -

julia> cltz.data |> extrema
(0.0f0, 12421.474f0)

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.

Thanks Akshay! The results look really nice. Just to clarify - the default remapping remains unchanged (spectral element), and now we have an option for bilinear remapping which can be used for e.g. diagnsotics?

@akshaysridhar
Copy link
Member Author

akshaysridhar commented Feb 18, 2026

Thanks Akshay! The results look really nice. Just to clarify - the default remapping remains unchanged (spectral element), and now we have an option for bilinear remapping which can be used for e.g. diagnsotics?

The intent here was to retain the existing SEM interpolation and add a simpler option that people can invoke elsewhere if necessary. Bilinear interpolation will "smooth" the result (but will not generate spurious negative values in the output due to ringing from higher polynomial orders). For diagnostic outputs only, I'd like to make the bilinear interpolation the default choice. I expect that, where conservation is critical we would be using the ConservativeRegridding tools rather than this Remapping module.

Copy link
Member

@imreddyTeja imreddyTeja 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 mostly good to me. I'm a bit confused by the interpolate_array changes though

end

"""`BilinearRemapping()` with no arguments: method tag; Remapper constructor fills in the arrays."""
BilinearRemapping() = BilinearRemapping(nothing, nothing, nothing, nothing)
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed? The constructor can also be dispatched to with the types

@juliasloan25
Copy link
Member

Btw the downstream ClimaLand tests are failing because of a separate issue that will be fixed today

@akshaysridhar
Copy link
Member Author

Btw the downstream ClimaLand tests are failing because of a separate issue that will be fixed today

Thanks - the build history issue also appears to be a broader permissions problem

@akshaysridhar akshaysridhar force-pushed the as/bilinear-horizontal-interp branch 2 times, most recently from 00717e5 to 26c716b Compare February 19, 2026 22:40
Add BilinearRemapping util to preserve existing interface

Co-authored-by: Julia Sloan <51397186+juliasloan25@users.noreply.github.com>
@akshaysridhar akshaysridhar force-pushed the as/bilinear-horizontal-interp branch from 26c716b to 31e34ad Compare February 19, 2026 22:42
@akshaysridhar akshaysridhar merged commit b846df0 into main Feb 20, 2026
35 checks passed
@akshaysridhar akshaysridhar deleted the as/bilinear-horizontal-interp branch February 20, 2026 02:47
imreddyTeja pushed a commit that referenced this pull request Feb 20, 2026
Simpler horizontal remapping methods for diagnostics
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