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

Rennovate notebook documentation #1

Closed
ThomasHaine opened this issue Nov 13, 2022 · 4 comments
Closed

Rennovate notebook documentation #1

ThomasHaine opened this issue Nov 13, 2022 · 4 comments

Comments

@ThomasHaine
Copy link
Collaborator

ThomasHaine commented Nov 13, 2022

Here are some comments (list of fixes/updates) on small demo.ipynb. These are intended to turn it into a more useful getting-started quickly demonstration.

  1. Add comments and markdown explaining what's going on. In particular, explain what [x,y,z,t] values you're interpolating, e.g. in cell 4.
  2. Label figures!
  3. Expand help message for OceInterp (cell 5).
  4. Explain why creating the masks (cell 6) is slow. Explain where the pre-computed masks are on SciServer (or add them to the repo?).
  5. Add keyword argument to set level of debugging/progress information as OceInterp runs (e.g., 100 left 98 left...) and improve clarity of messages. BTW The start time reported by the particle integrator doesn't exactly match the specified start time. Add message to clarify.
  6. Some more information about the derivative kernels is needed. And some information about the kernels near boundaries.
  7. Is it true that the API includes only 1 function (OceInterp)?
  8. What are the options for returned variables? I.e., fields in the raw variable. E.g., what's the particle depth?
@MaceKuailv
Copy link
Owner

The mask of U,V and W are generated like this:

The variables on the wall is only masked if both of the cells the wall is separating is masked, which means that the masked cells are examined almost one by one (I need to check again).

This does not make difference when the kernel diameter is smaller than 3 grid points and the mask value of velocity is zero or nan. I guess I can make a switch to skip this step if the kernel is too small. Thanks for drawing attention on this.

@ThomasHaine
Copy link
Collaborator Author

OK. This is a once-only calculation, right? I suggest adding to code to check if the files exist. If so, read them. If not, build them.

@MaceKuailv MaceKuailv changed the title small demo.ipynb comments Rennovate notebook documentation May 25, 2023
@MaceKuailv
Copy link
Owner

MaceKuailv commented May 25, 2023

This is a pretty old issue, and the notebook small_demo.ipynb is also replaced by newer ones. In a sense, this issue is already irrelevant. However, some of the stuff mentioned here is still lacking in my notebook documentation. Plus, this is the 1st issue.

I will use this issue as a hub for creating and ticking to-do lists. I will iterate until someday I close this issue. Someday, the documentation will be actually good. Yes, someday.

Many issues mentioned by @ThomasHaine is already resolved during the past year. Here are a few things that I still need to work on for existing notebooks.

  • Coherent text between code
  • Label of figures
  • Detailed explanation for less intuitive functionalities. Derivatives, __particle.raw, etc.

Here are a few wishlist notebooks that I want to make:

  • Demonstration on topology
  • Idealized example in the vertical.
  • Conservation of stream function example.

@MaceKuailv
Copy link
Owner

I am happy about the documentation.
#75 This is one of the last steps before I think it is ready to be submitted.

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

No branches or pull requests

2 participants