Add `Docs/Visualization` page + convert figs to vector form (`svg`)

[navidcy]

• adjusts some figures that were plotting transposed arrays by accident
• removes interpolate = true which allowed for mistakenly plotting rotated arrays without erroring.
• adds some remarks about unit conversions in examples

[navidcy]

Was there any particular reason interpolate = true kwarg was provided to heatmap?

[codecov[bot]]

Codecov Report

Patch and project coverage have no change.

Comparison is base (7ac0d4a) 52.91% compared to head (9301e61) 52.91%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #106 +/- ## ======================================= Coverage 52.91% 52.91% ======================================= Files 26 26 Lines 1030 1030 ======================================= Hits 545 545 Misses 485 485 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[jagoosw]

Was there any particular reason interpolate = true kwarg was provided to heatmap?

Not really, I think it was just because it looks nicer sometimes

[jagoosw]

Its not from this PR but I've just noticed that the air/sea CO2 flux and sinking flux are the same line in the column (and probably other) examples, I think because this line: https://github.com/OceanBioME/OceanBioME.jl/blob/e7bd802d853c8e5118bb4510506f1b8f81b0a1c2/examples/column.jl#L99

Doesn't create two separate arrays but references one with both variables. I think it has to be something like:

air_sea_CO₂_flux, carbon_export = zeros(length(times)),  zeros(length(times)) 

(they shouldn't be the exact same but average to the same over a long time because the air/sea exchange is slow)

[navidcy]

Its not from this PR but I've just noticed that the air/sea CO2 flux and sinking flux are the same line in the column (and probably other) examples, I think because this line:

https://github.com/OceanBioME/OceanBioME.jl/blob/e7bd802d853c8e5118bb4510506f1b8f81b0a1c2/examples/column.jl#L99

Doesn't create two separate arrays but references one with both variables. I think it has to be something like:

air_sea_CO₂_flux, carbon_export = zeros(length(times)),  zeros(length(times)) 

(they shouldn't be the exact same but average to the same over a long time because the air/sea exchange is slow)

You are correct!

julia> a = b = zeros(3)
3-element Vector{Float64}:
 0.0
 0.0
 0.0

julia> for j in 1:3; a[j]=j; end

julia> a
3-element Vector{Float64}:
 1.0
 2.0
 3.0

julia> b
3-element Vector{Float64}:
 1.0
 2.0
 3.0

[navidcy]

One this that is not explained is why each of these are computed at different depths? Also what depth this computation is made is never mentioned...

E.g. below air_sea_CO₂_flux at k = 50 while carbon_export at k = end-20.

    air_sea_CO₂_flux[i] = CO₂_flux.condition.parameters(0.0, 0.0, t, DIC[1, 1, 50, i], Alk[1, 1, 50, i], temp(1, 1, 0, t), 35)
    carbon_export[i] = (sPOM[1, 1, end-20, i] * model.biogeochemistry.sinking_velocities.sPOM.w[1, 1, end-20] +
                        bPOM[1, 1, end-20, i] * model.biogeochemistry.sinking_velocities.bPOM.w[1, 1, end-20]) * model.biogeochemistry.organic_redfield

[navidcy]

Also this * (12 + 16 * 2) * year / 1e6: I get that is some sort of unit conversion but 12+16*2 does not ring any bells.

[jagoosw]

I should probably add some description to this. Basically the air-sea flux is at the surface (maybe should change 50 to grid.Nz), and the sinking flux is how much sinks below some arbitrary depth (I think 100m here).

The unit conversion is mmol N to kg CO2 so the 12 + 2x16 is for the mass of carbon dioxide

[navidcy]

I added something. Feel free to modify/edit/push on the branch!

[navidcy]

@jagoosw any idea why this happens?

https://github.com/OceanBioME/OceanBioME.jl/actions/runs/5115423233/jobs/9196670493?pr=106#step:5:725

[jagoosw]

I'm not sure why this happens sometimes with the Eady example. That error is from the phytoplankton going negative but I'm not sure how that happens with:

simulation.callbacks[:neg] = Callback(scale_negative_tracers; callsite = UpdateStateCallsite())
simulation.callbacks[:nan_tendencies] = Callback(remove_NaN_tendencies!; callsite = TendencyCallsite())
simulation.callbacks[:abort_zeros] = Callback(zero_negative_tracers!; callsite = UpdateStateCallsite())

I'll re-run it and if that doesn't work try and work out what causes it locally.

[johnryantaylor]

Lowering the CFL number is a good idea. Another approach would be to update the CFL criterion every timestep instead of every 10 iterations as it is currently done. The Eady problem is highly frontogenetic and the flow can change very rapidly - hence a timestep that was stable a few iterations ago might become too large.

On May 30, 2023, at 3:04 PM, Gregory L. Wagner @.***> wrote:

@glwagner commented on this pull request.

In examples/eady.jl https://github.com/OceanBioME/OceanBioME.jl/pull/106#discussion_r1210328550:

@@ -75,7 +75,7 @@ set!(model, u=uᵢ, v=vᵢ, P = 0.03, Z = 0.03, NO₃ = 4.0, NH₄ = 0.05, DIC = simulation = Simulation(model, Δt = 15minutes, stop_time = 10days)

Adapt the time step while keeping the CFL number fixed

-wizard = TimeStepWizard(cfl=0.85, max_change = 1.5, max_Δt = 30minutes) +wizard = TimeStepWizard(cfl=0.80, max_change = 1.5, max_Δt = 30minutes) ⬇️ Suggested change -wizard = TimeStepWizard(cfl=0.80, max_change = 1.5, max_Δt = 30minutes) +wizard = TimeStepWizard(cfl=0.5, max_change = 1.5, max_Δt = 30minutes) Wide margins are better...

— Reply to this email directly, view it on GitHub https://github.com/OceanBioME/OceanBioME.jl/pull/106#pullrequestreview-1451049587, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE3E34IY44NHWJ55OAWKSMTXIX45ZANCNFSM6AAAAAAYRTGZ44. You are receiving this because you are subscribed to this thread.

[jagoosw]

I can't get it to reliably run now even with a lower cfl, could it be the change to the diffusion?

[glwagner]

I can't get it to reliably run now even with a lower cfl, could it be the change to the diffusion?

Hmm not sure but we find that WENO typically provides sufficient horizontal dissipation, so you may want to eliminate explicit horizontal diffusion altogether. Most users shouldn't use Laplacian horizontal diffusion anyways for this kind of problem...

[jagoosw]

Removing the horizontal diffusion makes it run again so I think it is that