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

improved overturn documentation and output conventions #80

Merged
merged 14 commits into from
Oct 25, 2021

Conversation

hdrake
Copy link
Contributor

@hdrake hdrake commented Oct 16, 2021

I had to comment out the css overwrites because I think they were completely overwriting the entire default css sheet.

Based on what it says here (https://www.sphinx-doc.org/en/master/usage/configuration.html) it seems that `CSS @import` is the correct way of doing small overwrites of the default css sheet.
- Added an info box explaining the basic idea behind Thorpe Scale methods for estimating dissipation rates

- Made Thorpe displacements more intuitive by unsorting them, so that the displacement is relative to the initial unsorted profile
- Importantly, this doesn't change any calculations!
- Added the sorting indices `sidx` to the diagnostics as well
- Made some small changes to the notebook to reflect these changes
@hdrake
Copy link
Contributor Author

hdrake commented Oct 16, 2021

The main thing I did was added this info box about the Thorpe scale method. There are also some other minor improvements, including changing the convention for outputting thorpe displacements (by unsorting)

Screen Shot 2021-10-16 at 6 47 39 PM

@hdrake
Copy link
Contributor Author

hdrake commented Oct 16, 2021

Note: 8c5b7d5 seems to have also broken the documentation formatting on master, but this Pull Request fixes it.

@jessecusack
Copy link
Collaborator

jessecusack commented Oct 16, 2021

Looks great and almost ready to go.

Would you mind running the notebooks from top to bottom with restarted kernel, then clear all the outputs, then commit the result to this PR. That should (hopefully) prevent some of the jupyter notebook metadata/binary bloat from creeping in.

@jessecusack
Copy link
Collaborator

In fact, running from top to bottom is not what I used to do... somehow I completely removed execution numbers. I have forgotten how though... this isn't a very big deal though. The figure data is the priority.

There are steps in which the shear/strain spectra arrays are sub-sampled:
1) using `m_include_sh` or `m_include_st` to zoom in on the spectral window specified by the user
2) using `iimsh` or `iimst` to cutoff high wavenumbers.

Previously, the `iim*` erroneously refered to the indices of arrays that were already indexed by `m_include_*` and yet were later erroneously used to index the full arrays (e.g. `m[iimsh]`). This is typically not a problem for shear because the first index is usually 0 (the window scale) and thus `iimsh` is equal to `m_include_sh[iimsh]`. However, for strain the first index is typically strictly greater than 0, so `iimst` is almost never equal to `m_include_st[iimst]`.

Essentially what this means is that strain variance was typically being integrated over a spectral window of much larger scales than specified by the user, and was thus giving erroneous results (I am not exactly sure the theoretical reason that strain is meant to be evaluated over higher wave numbers than shear, but Kunze recommends a min wavenumber that corresponds to a max wavelength of 150 m. In the documentation example, this was erroneously being set to 300 m by this bug). This bug was not as bad as it could have been because it was applied to both the GM and observed spectra, and so the errors cancelled out to some extent.
- I made the formulas for calculating the final dissipation rates into their own functions (one for shearstrain and one for strain-only). I also explicitly related the diffusivity expression to the dissipation rate function, rather than hard-coding both.

- I added diags for Nm and the corrected shear/strain ratio (with non-physical values of Rw<1 set to 1.01) which both should probably be accessible to users for basic quality control and data exploration.
Previously krho and eps were separately estimated relative to GM reference levels. This is fine as long as the reference K0, eps0, and N0 are always self-consistent, but it is unecessarily redundant and prone to user error. I found, for example, that the K0 used was consistent with the eps0 from Gregg 2003 but inconsistent with the present default value from Waterman 2014, giving errors of ~15% in the final diffusivities.

I've fixed the code by implementing a separate `diffusivity` function which compute the diffusivity from the dissipation rate, the stratification, and a mixing coefficient (set to Gam=0.2 by default, as in Gregg 2003 and others). Now all one needs to do is change eps0 and the diffusivity will always be self-consistent.
This are the final diagnostics required for users to recreate the final dissipation profile based on profiles of each individual term (stratification, shear or strain variance, and correction terms for aspect ratio and Coriolis frequency).
- Includes an info panel the summarizes the theory and practice of internal wave finescale paramterization
- Includes a basic explanation of the integration window parameters and prints their values in physical wavelength units
- Includes all of the bug fixes in earlier commits
- Includes plots of more granular diagnostics, both in terms of spectra for a given depth window and also for full vertical profiles
- Uses more intuitive `plt.stairs` plotting convention than misleading `plt.steps`
- Declutters some of the plotting code
@hdrake
Copy link
Contributor Author

hdrake commented Oct 24, 2021

This and #82 are now ready to go. I already merged them both so you can merge either into master and then close the other.

@hdrake hdrake changed the title Improve Thorpe Scale documentation overturn and helper bugfixes and improved documentation Oct 24, 2021
@hdrake hdrake changed the title overturn and helper bugfixes and improved documentation overturn (and helper) bugfixes and improved documentation Oct 24, 2021
@hdrake hdrake changed the title overturn (and helper) bugfixes and improved documentation improved overturn documentation Oct 24, 2021
@hdrake hdrake changed the title improved overturn documentation improved overturn documentation and overturn output conventions Oct 24, 2021
@hdrake hdrake changed the title improved overturn documentation and overturn output conventions improved overturn documentation and output conventions Oct 24, 2021
- add docstring for diffusivity
- add diffusivity, aspect_ratio_correction_shst, aspect_ratio_correction_st to api
@gunnarvoet
Copy link
Member

gunnarvoet commented Oct 25, 2021

I went through the changes in shearstrain and they all look great! I am very glad you found the indexing bug and agree with your comment in #60 that the user input should be the wavenumbers instead of indexing a wavenumber range.

The documentation has improved a lot with your edits! I went through and just cleaned up the code a bit for better readability. I also added your new functions in shearstrain to the API which has to done manually.

For reference - all jupyter notebook output should be cleaned prior to committing changes. That way, we do not have any images in the diffs. Sphinx runs the notebooks when it produces the documentation.

I will go ahead and merge this pull request and close #82 .

@hdrake
Copy link
Contributor Author

hdrake commented Oct 25, 2021

For reference - all jupyter notebook output should be cleaned prior to committing changes. That way, we do not have any images in the diffs. Sphinx runs the notebooks when it produces the documentation.

Ah, okay thanks that makes sense!

I will go ahead and merge this pull request and close #82 .

Great!

@gunnarvoet gunnarvoet merged commit 7a8f4e0 into modscripps:master Oct 25, 2021
@hdrake hdrake deleted the improve-thorpe-documentation branch October 25, 2021 11:11
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.

None yet

3 participants