gotm-model / code

Source code for the General Ocean Turbulence Model
https://gotm.net
GNU General Public License v2.0
53 stars 44 forks source link

Langmuir #33

Closed knutaros closed 2 years ago

knutaros commented 2 years ago

This branch adds the Stokes Coriolis force and additional Langmuir production terms to the various turbulence models.

bolding commented 2 years ago

We will need to update the examples as well to be compatible

bolding commented 2 years ago

Knut, Lars - what shall the commit message be?

lumlauf commented 2 years ago

For the commit message I would suggest: "Adding Kantha and Clayson (2004) Langmuir parameterization to all two-equation models".

bolding commented 2 years ago

gotm-cases contains the 'default'-versions of gotm.yaml. As the default does not contain the parameters for the different turbulence models we do not need to re-generate gotm.yaml. I've tested - and with - --detail full - ce4 is included with a value of 0

bolding commented 2 years ago

The Github actions - that automatically builds GOTM and runs a few cases fails

image

Thought you said stokes_drift was always included

lumlauf commented 2 years ago

Strange, I can compile and run it here without problems. Knut, do you see what might have gone wrong?

jornbr commented 2 years ago

meanflow/coriolis appears to depend on stokes_drift. In that case, https://github.com/gotm-model/code/blob/705216694dbc43cee2ea58c4b9a131a7a13f5efd/src/meanflow/CMakeLists.txt#L18 needs an additional dependency on stokes_drift. Without that, meanflow is allowed to be compiled before stokes_drift, causing the problem seen here.

bolding commented 2 years ago

is there a good reason why stake drift is not in the meanflow folder?

lumlauf commented 2 years ago

I remember we discussed this with Qing during the coupling of CVMix and GOTM. One argument was that waves are neither "meanflow" nor "turbulence" - so they should go to a neutral place.

bolding commented 2 years ago

OK - but as we just saw compilation became more complicated - and we treat the Stokes drift more like mean flow by e.g. adding to u and v. Maybe usprof and vsprof should go to observations instead of being from stokes_drift.F90 - like tprof, sprof etc. Independent if they stay or go I think dusdz, dvsdz should be parameters to shear().

lumlauf commented 2 years ago

Accessing dusdz and dvsdz by a use statement in gotm.F90, and then passing them as arguments to "shear()" makes sense, I'd say. This also corresponds to the data flow in Fig. 1 of our CVMix paper (see attachment).

Regarding usprof and vsprof, I'm not so sure. The stokes drift module seems the natural place for them. What would be the motivation for moving them to a different place? I find this a bit counter-intuitive.

stokes

knutaros commented 2 years ago

thanks a lot for spotting. fixed. strange, that the cmake logic on my system generated a different sequence such that this error does not pop up...

On 6/19/22 17:10, Jorn Bruggeman wrote:

meanflow/coriolis appears to depend on stokes_drift. In that case, https://github.com/gotm-model/code/blob/705216694dbc43cee2ea58c4b9a131a7a13f5efd/src/meanflow/CMakeLists.txt#L18 needs an additional dependency on stokes_drift. Without that, meanflow is allowed to be compiled before stokes_drift, causing the problem seen here.

— Reply to this email directly, view it on GitHub https://github.com/gotm-model/code/pull/33#issuecomment-1159751489, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2RV6XLVHZ47OCR3DLKIWDVP4Z43ANCNFSM5ZDCRXIA. You are receiving this because you authored the thread.Message ID: @.***>

knutaros commented 2 years ago

good, but could clarify a bit the logic behind the different display_advanced selections?

Take for example the entrainment yaml. There len_scale_method=dissipation, but still the yaml contains the generic section. Why?

On 6/19/22 16:27, Karsten Bolding wrote:

gotm-cases contains the 'default'-versions of gotm.yaml. As the default does not contain the parameters for the different turbulence models we do not need to re-generate gotm.yaml. I've tested - and with

  • --detail full - ce4 is included with a value of 0

— Reply to this email directly, view it on GitHub https://github.com/gotm-model/code/pull/33#issuecomment-1159740390, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2RV6UQKPQSLKYKU6TSZQTVP4U27ANCNFSM5ZDCRXIA. You are receiving this because you authored the thread.Message ID: @.***>

bolding commented 2 years ago

There are no handling of conditions between different sections in the yaml-file - only if a variable is to be shown or not.

bolding commented 2 years ago

error in compilation - if you did not clean everything you might not see the error as the dependency might have been build - and even if you did the cmake build path is not predetermined - especially if you build in parallel

bolding commented 2 years ago

If the API for shear is changed so should the interface for coriolis() - then meanflow would not depend on stokes_drift

bolding commented 2 years ago

Did we not talk about passing parameters to e.g. coriolis() to avoid having meanflow depending on stokes_drift?

lumlauf commented 2 years ago

Yes, we did. We plan to do this soon, when we continue with the implementation of the more complex models for Langmuir turbulence by Ramsey.

On 7/15/2022 15:19, Karsten Bolding wrote:

Dis we not talk about passing parameters to e.g. coriolis() to avoid having meanflow depending on stokes_drift?

— Reply to this email directly, view it on GitHub https://github.com/gotm-model/code/pull/33#issuecomment-1185538229, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEVOCRNZG6AMKL4FOGJWRLDVUFQMPANCNFSM5ZDCRXIA. You are receiving this because you commented.Web Bug from https://github.com/notifications/beacon/AEVOCRMHCZDUVQ7BGPY7PQDVUFQMPA5CNFSM5ZDCRXIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOI2U6BNI.gifMessage ID: @.***>

[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/gotm-model/code/pull/33#issuecomment-1185538229", "url": "https://github.com/gotm-model/code/pull/33#issuecomment-1185538229", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

--

Lars Umlauf

Physical Oceanography and Instrumentation Leibniz-Institute for Baltic Sea Research

phone : ++49 381 5197 223 fax : ++49 381 5197 114 web :www.io-warnemuende.de/lars-umlauf-en.html

address: Leibniz-Institute for Baltic Sea Research Seestrasse 15 D-18119 Rostock-Warnemuende Germany

knutaros commented 1 year ago

On 6/19/22 19:59, lumlauf wrote:

Accessing dusdz and dvsdz by a use statement in gotm.F90, and then passing them as arguments to "shear()" makes sense, I'd say. This also corresponds to the data flow in Fig. 1 of our CVMix paper (see attachment).

The argument behind providing dudsz as argument is to avoid USE'ing non-meanflow variables in shear? Indeed, would be solved by having stokes profiles defined in meanflow :-)

Regarding usprof and vsprof, I'm not so sure.

I would not move them to observations, as they could also be approximated internally from wind data. Not sure, whether these secondary variables should also be counted as "primary observations"...

The stokes drift module seems the natural place for them. What would be the motivation for moving them to a different place? I find this a bit counter-intuitive.

Are there special design considerations that argue against having a stokes_drift module (or a more general wave module) inside the meanflow directory, such that 1) the wave/stokes_drift stuff is still logically separated but also 2) the stokes_drift variables can be accessed via "use meanflow" (because the meanflow module contains "use stokes_drift")?

stokes https://user-images.githubusercontent.com/19587397/174494226-62eeebda-dc1b-4d9e-8518-820a90e75c62.png

— Reply to this email directly, view it on GitHub https://github.com/gotm-model/code/pull/33#issuecomment-1159784693, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2RV6S65JH6EHYADF37ZK3VP5NWPANCNFSM5ZDCRXIA. You are receiving this because you authored the thread.Message ID: @.***>