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

add a unit test for background flux divergence and clean up unrelated commits #4028

Open
wants to merge 9 commits into
base: glw/background-flux-divergence
Choose a base branch
from

Conversation

liuchihl
Copy link
Contributor

@liuchihl liuchihl commented Jan 5, 2025

This PR adds a unit test for #3646

The tests compare model results with and without background fields using a linear stratification profile.

@liuchihl
Copy link
Contributor Author

liuchihl commented Jan 5, 2025

@glwagner, this PR has been updated and aligned with the target branch.

@liuchihl liuchihl changed the title add a unit test for background flux divergence and clean up unnecessary commits add a unit test for background flux divergence and clean up unrelated commits Jan 5, 2025
Updates the previous unit test to a more robust correctness test. Comparing total buoyancy values with and without background flux with no-flux boundary at the bottom and assumes infinite ocean at the top boundary.
@liuchihl
Copy link
Contributor Author

liuchihl commented Jan 7, 2025

This unit test has been updated to a more robust correctness test.
Here is a summary of this test:
The test compares two model runs: one with background fields and one without. The model with background fields uses a BackgroundField for buoyancy, while the model without background fields incorporates the stratification as an initial condition. The test then compares the total buoyancy fields (B) from both runs. This verifies that the background field implementation produces consistent results with the traditional approach of including background stratification in the initial conditions.

schedule = IterationInterval(1),
verbose=true,
filename = filepath,
overwrite_existing = true)
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to use output here? I think you can directly return the value of B without output. That will make the test simpler and easier to maintain which is super important.

@glwagner
Copy link
Member

glwagner commented Jan 7, 2025

Main feedback is to set up the test without using an output writer which I don't think is necessary for this test. This will make the test easier to maintain.

Once that is done let me know and I will run this manually and then merge it.

@liuchihl
Copy link
Contributor Author

liuchihl commented Jan 7, 2025

Thanks, @glwagner for reviewing this!
I agree with you that it is best to avoid outputwriter in a unit test, but I have trouble reading B:

julia> b = model.tracers.b
1×1×10 Field{Center, Center, Center} on RectilinearGrid on CPU
├── grid: 1×1×10 RectilinearGrid{Float64, Flat, Flat, Bounded} on CPU with 0×0×3 halo
├── boundary conditions: FieldBoundaryConditions
│   └── west: Nothing, east: Nothing, south: Nothing, north: Nothing, bottom: Gradient, top: Gradient, immersed: ZeroFlux
└── data: 1×1×16 OffsetArray(::Array{Float64, 3}, 1:1, 1:1, -2:13) with eltype Float64 with indices 1:1×1:1×-2:13
    └── max=0.0, min=0.0, mean=0.0

julia>= model.background_fields.tracers.b
FunctionField located at (Center, Center, Center)
├── func: constant_stratification (generic function with 1 method)
├── grid: 1×1×10 RectilinearGrid{Float64, Flat, Flat, Bounded} on CPU with 0×0×3 halo
├── clock: Clock{Float64, Float64}(time=0 seconds, iteration=0, last_Δt=Inf days)
└── parameters: (N² = 1.0e-6,)

julia> B =+ b 
BinaryOperation at (Center, Center, Center)
├── grid: 1×1×10 RectilinearGrid{Float64, Flat, Flat, Bounded} on CPU with 0×0×3 halo
└── tree: 
    + at (Center, Center, Center)
    ├── 1×1×10 Oceananigans.Fields.FunctionField{Center, Center, Center, Clock{Float64, Float64}, @NamedTuple{N²::Float64}, typeof(constant_stratification), RectilinearGrid{Float64, Flat, Flat, Bounded, Oceananigans.Grids.StaticVerticalCoordinate{OffsetArrays.OffsetVector{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}, Float64}, Float64, Float64, Nothing, Nothing, CPU}, Float64}
    └── 1×1×10 Field{Center, Center, Center} on RectilinearGrid on CPU

As shown, B becomes a BinaryOperation when combining the background and perturbation buoyancy. I have trouble reading B properly without using outputwriter. This contrasts with Field, where I can simply utilize the interior function. Do you have suggestions on how to do this?

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.

2 participants