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

Docs: simplify notation + update Docs/Forcing section + remove Ito comments from modules/tests #178

Merged
merged 34 commits into from
Dec 30, 2020

Conversation

navidcy
Copy link
Member

@navidcy navidcy commented Dec 26, 2020

This PR

  • simplifies notation in Docs, e.g. \mathrm{e}^{\mathrm{i} k x} -> e^{i k x}.
  • updates Docs/Stochastic Forcing section and incorporates code to generate plots.
  • removes all comments from modules/tests regarding Ito vs Stratonovich calculi.
  • Enhance various docstrings + update MultilayerQG module description in Docs.

Closes #117.

To preview the docs: https://fourierflows.github.io/GeophysicalFlowsDocumentation/previews/PR178/

@navidcy navidcy changed the title simplify notation + smooth Docs/Forcing + remove Ito comments Docs: simplify notation +update Docs/Forcing section + remove Ito comments from modules/tests Dec 26, 2020
@navidcy navidcy changed the title Docs: simplify notation +update Docs/Forcing section + remove Ito comments from modules/tests Docs: simplify notation + update Docs/Forcing section + remove Ito comments from modules/tests Dec 26, 2020
@@ -561,11 +606,12 @@ is the number of layers in the fluid. (When ``n=1``, only the kinetic energy is

The kinetic energy at the ``j``-th fluid layer is
```math
\\textrm{KE}_j = \\frac{H_j}{H} \\int \\frac1{2} |\\boldsymbol{\\nabla} \\psi_j|^2 \\frac{\\mathrm{d}^2 \\boldsymbol{x}}{L_x L_y} \\ , \\quad j = 1, \\dots, n \\ ,
𝖪𝖤_j = \\frac{H_j}{H} \\int \\frac1{2} |{\\bf ∇} ψ_j|^2 \\frac{𝖽x 𝖽y}{L_x L_y} = \\frac1{2} \\frac{H_j}{H} \\sum_{𝐤} |𝐤|² |ψ̂_j|², \\ j = 1, ..., n \\ ,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the simplifications you've achieved here.

I have one very small comment. I noticed that there is some notation that is not simple roman letters. This makes sense for math notation and vectors, but is much more subtle when (unicode?) roman letters are used in place of normal roman letters. For example in this equation I believe KE and differential d are unicode, but this was only apparent to me through the font changes in the rendered equations (see below). I think that is okay for now, but it may be useful to make unicode usage more obvious (is that even possible?) if a wider group of people start modifying the docs.

Screen Shot 2020-12-28 at 9 10 54 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I mostly chose to use unicode because then the docstrings look nice in the REPL... Otherwise you see things like, e.g. \textrm{KE}.

But you make a nice point. I'll raise an issue to have a small section "Contributors" or something like that and add this clarification...

@BrodiePearson did you have a look at the Stochastic Forcing section? I'm still not 100% satisfied, e.g. a definition of W_t should appear. But at some point one needs to stop since we are not writing a book here... :)

Copy link
Member Author

Choose a reason for hiding this comment

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

(see #180)

@navidcy
Copy link
Member Author

navidcy commented Dec 30, 2020

I'll merge and we can revisit "Stochastic Forcing" section again later on...

@navidcy navidcy merged commit 0f565c8 into master Dec 30, 2020
@navidcy navidcy deleted the ncc/docs-simplify branch February 25, 2021 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up comments regarding work done by stochastic forcing
2 participants