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

Fixes to KPP interface #107

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

vanroekel
Copy link
Collaborator

Two portions of the KPP interface had bugs

  1. the computation of surface buoyancy forcing and friction velocity were incorrect. The surfaceBuoyancyForcing subtracted the wrong values from the heat fluxes from thickness fluxes. The surfaceFrictionVelocity had incorrect interpolation to centers

  2. the KPP interface for matchBoth is fixed and the indexing is fixed for matching interior to boundary layer diffusivities.

Two portions of the KPP interface had bugs
1. the computation of surface buoyancy forcing and friction velocity
were incorrect.  The surfaceBuoyancyForcing subtracted the wrong values
from the heat fluxes from thickness fluxes.  The surfaceFrictionVelocity
had incorrect interpolation to centers

2. the KPP interface for matchBoth is fixed and the indexing is fixed
for matching interior to boundary layer diffusivities.
@vanroekel vanroekel added the bug Something isn't working label Jul 10, 2024
@vanroekel vanroekel requested a review from cbegeman July 10, 2024 20:49
@vanroekel
Copy link
Collaborator Author

@cbegeman - here is the branch I mentioned in slack -- @katsmith133 and @maltrud I'd appreciate your look as well on these proposed fixes.

Sorry for the rough commit history. I put this on top of e3sm master, which is ahead of ocean discussions.

Copy link

github-actions bot commented Jul 10, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://E3SM-Ocean-Discussion.github.io/E3SM/pr-preview/pr-107/
on branch gh-pages at 2024-07-12 21:19 UTC

@xylar xylar changed the base branch from master to alternate July 11, 2024 15:36
@xylar xylar changed the base branch from alternate to master July 11, 2024 15:36
@@ -65,7 +65,7 @@ module ocn_vmix_cvmix
type(cvmix_tidal_params_type) :: cvmix_tidal_params

logical :: cvmixOn, cvmixConvectionOn, cvmixKPPOn
real (kind=RKIND) :: backgroundVisc, backgroundDiff
real (kind=RKIND) :: multVal, backgroundVisc, backgroundDiff
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vanroekel Searching for this value doesn't bring up anything for me. Where is it assigned?

@cbegeman
Copy link
Collaborator

@vanroekel Let me know when this branch is ready for testing. I was thinking we should set up idealized tests, polaris's ocean/single_column/cvmix, with the same buoyancy forcing applied in each case but applied using different forcing terms.

@vanroekel
Copy link
Collaborator Author

@cbegeman the branch is compiling but only running a couple steps before crashing in a global LR test. Do you want me to push what I have so you can test too?

@cbegeman
Copy link
Collaborator

@vanroekel My testing could be helpful. It would indicate whether the branch can run in a very idealized standalone case. If you agree, push and ping me.

@vanroekel
Copy link
Collaborator Author

@cbegeman I just pushed a branch that is timestepping now. I'd definitely appreciate you taking it for a test run.

@cbegeman
Copy link
Collaborator

Testing with polaris's ocean/single_column/cvmix test, 5 day simulation:

  • results with rain and river fluxes of same magnitude are identical with master and this branch (@vanroekel Is this expected? I thought there would be a difference between the two with master if it was buggy)
  • T,S gradients at base of ML are smoother with this branch (bottom) than with master (top)
    temperature
    temperature

@vanroekel
Copy link
Collaborator Author

  • results with rain and river fluxes of same magnitude are identical with master and this branch (@vanroekel Is this expected? I thought there would be a difference between the two with master if it was buggy)

good question. I think it would depend on the magnitude. If the river forcing is quite strong it is possible that the freshwater signal dominates the changes in surface buoyancy from temperature. I'm guessing this may be the case given the strong negative temperature gradient shown in your figures.

@cbegeman
Copy link
Collaborator

good question. I think it would depend on the magnitude. If the river forcing is quite strong it is possible that the freshwater signal dominates the changes in surface buoyancy from temperature. I'm guessing this may be the case given the strong negative temperature gradient shown in your figures.

Would the best way to get a signal be to set the temperature to a uniform value (and then master should have an unphysical temperature evolution whereas this branch will be constant)?

@vanroekel
Copy link
Collaborator Author

vanroekel commented Jul 15, 2024

I think what we would need is a case where

alpha*surfaceThicknessFlux*temperature_sfc > beta*surfaceThicknessFlux*salinity_sfc

so perhaps a case would be to use the linear EOS and artificially reduce beta? Or choose a VERY fresh salinity? I think in this case master would overmix relative to this branch

Creating a uniform T and forcing S will actually trigger a really weird instability in KPP. It is known (not widely) that if you have a well mixed T and force salinity at the surface KPP will artificially create gradients in T.

@cbegeman
Copy link
Collaborator

I think what we would need is a case where

alphasurfaceThicknessFluxtemperature_sfc > betasurfaceThicknessFluxsalinity_sfc
so perhaps a case would be to use the linear EOS and artificially reduce beta? Or choose a VERY fresh salinity? I think in this case master would overmix relative to this branch

Creating a uniform T and forcing S will actually trigger a really weird instability in KPP. It is known (not widely) that if you have a well mixed T and force salinity at the surface KPP will artificially create gradients in T.

I ran this test with linear EOS and beta greatly reduced such that this condition is true and I'm still not seeing significant differences with rain or river forcing. Not saying that necessarily indicates a problem; just thought you should know.

@vanroekel
Copy link
Collaborator Author

very interesting @cbegeman thanks for letting me know. I'll think about what this test means.

@vanroekel
Copy link
Collaborator Author

@cbegeman a quick update on this. I've looked at the behavior in a single column config and see the same as you. I'm not seeing much sensitivity to the nonlocal changes, which is intriguing. Could you rerun your single column config but backing out the changes to mpas_ocn_vmix_cvmix.F? I'm curious if that changes the smoothing at the boundary layer base you see.

@cbegeman
Copy link
Collaborator

@vanroekel I reverted just the changes to mpas_ocn_vmix_cvmix and am still seeing the smoothing at the base of the mixed layer, which is not what I expected.

@vanroekel
Copy link
Collaborator Author

Thanks @cbegeman that is consistent with what I saw too, and I don't understand it. I'll dig into this further.

@vanroekel
Copy link
Collaborator Author

@cbegeman I'm working on this again and wondered if you could pass along the test you conducted (a polaris branch or just settings for stratification and forcing). I'd like to narrow down what is causing the smoother BL base.

@cbegeman
Copy link
Collaborator

@vanroekel I was running this polaris test case ocean/single_column/cvmix with this branch https://github.com/cbegeman/polaris/tree/enhance-cvmix-test. To test different surface flux types, you just change the values in cvmix.cfg. I was just taking the default value for evap and moving it to river and rain fluxes.

@vanroekel
Copy link
Collaborator Author

@cbegeman I figured out why the new PR is more diffusive at the base of the ML. The single column test case uses the MatchBoth matching option. The current setup of that configuration is less diffusive at the base, but also allows for negative diffusivities to emerge. This PR fixes that but also diffuses the base of the boundary layer more. Here is a comparison top is master, bottom is PR

salinity

salinity-ss

@vanroekel
Copy link
Collaborator Author

so this looks very similar. In this PR the biggest change comes from a switch to ParabolicNonLocal, shown here
salinity-pnl

@vanroekel
Copy link
Collaborator Author

What are your thoughts on best next steps for this? From this testing, I think the effect of these changes is small, but they are bugs, so should be fixed. Maybe a G-case? Or should we run a B-case right away?

@cbegeman
Copy link
Collaborator

@vanroekel Great! I think since we are expecting the changes to be small we can move right to a B-case.

@vanroekel
Copy link
Collaborator Author

One other thing I forgot to call out here, but I also fixed the interface to allow us to use a different config of cvmix. This one actually makes some difference

image

I'm setting up the baroclinic gyre now to start testing further before going to a B-case. I was thinking of finding the 'best' CVMix config in the gyre case and then do a B-case of that. Does that seem reasonable to you?

@cbegeman
Copy link
Collaborator

The baroclinic gyre seems like a good test. If the cvmix config makes a difference to the baroclinic gyre strength though, then maybe a G-case would be useful to isolate impacts on AMOC.

@vanroekel
Copy link
Collaborator Author

@cbegeman here is MPAS-Analysis from 50 years of a coupled test with these fixes
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.vanroekel/E3SMv3/20240918_kppTest_ne30pg2_icos30_anvil/mpas_analysis/ts_0001-0050_climo_0001-0050/
Here is the baseline
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.wlin/E3SMv3/20231209.v3.LR.piControl-spinup.chrysalis/mpas_analysis/ts_0001-0050_climo_0001-0050/

I haven't looked at these very closely yet, but from a very quick scan the changes are pretty minor, there are some increases in high latitude bias (slightly colder sst, fresher, and slightly weaker amoc, ice thickness is also up a bit but seems to be trending down at the end of the 50 years). The drake transport seems better with the fixes. I can post e3sm-diags when they finish

@cbegeman
Copy link
Collaborator

@vanroekel I agree. The changes look small enough that we should just proceed with opening this PR to E3SM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants