Comments on notebooks about cfdpython HOT 16 CLOSED

05_Step_4

• Cell [3], I think that it would be more consistent to use sympy.pi instead of numpy.py
phi = (sympy.exp(-(x - 4 * t)**2 / (4 * nu * (t + 1))) + sympy.exp(-(x - 4 * t - 2 * numpy.pi)**2 / (4 * nu * (t + 1)))) >> phi = (sympy.exp(-(x - 4 * t)**2 / (4 * nu * (t + 1))) + sympy.exp(-(x - 4 * t - 2 * sympy.pi)**2 / (4 * nu * (t + 1))))

from cfdpython.

06_Array_Operations_with_NumPy

• After Cell [6]: it might be useful to mention that the running time depends on the computer. For some people it might be obvious, but that's not always the case.

from cfdpython.

13_Step_10

• Before cell [1], I would change b_{i,j}=-100$ at $i=nx*3/4, j=3/4 ny for b_{i,j}=-100$ at $i=3/4 nx, j=3/4 ny

from cfdpython.

01_Step_1

• ipython notebook >> jupyter notebook

from cfdpython.

02_Step_2

• Cell [1],
  import numpy #we're importing numpy and calling it np locally
  NumPy is called numpy and not np. Same goes for matplotlib.pyplot.

from cfdpython.

14_Optimizing_Loops_with_Numba

• The notebook mixes spaces and tabs.

• laplace2d functions mix the alias for NumPy, using numpy and np. Due to this the notebook does not work, the following is the returned error

  NameError: name 'np' is not defined

Clearly the NumPy + Numba combination is the fastest, but the ability to quickly optimize code with nested loops can also come in very handy in certain applications.

This is not the case in my computer. I have 91.1 µs ± 40.2 µs in the NumPy + Numba case and 62.5 µs ± 30.8 µs in the Vanilla + Numba.

from cfdpython.

15_Step_11

It might be a good idea to consider streamplots besides glyphs for the vector visualization. It suffices to change

pyplot.quiver(X, Y, u, v)

for

pyplot.streamplot(X, Y, u, v)

For some vector fields this is easier to understand, and introduces another visualization technique.

This comment is a matter of taste, so I understand if the authors don't consider it needed.

from cfdpython.

17_NumbaPro

• The extension for this notebook has a .bak appended.

• Cell [1] returns the following warning

  numbapro:1: ImportWarning: The numbapro package is deprecated in favour of the accelerate package. Please update your code to use equivalent functions from accelerate.

• Cell [1] returns the following error

  from numbapro import autojit, cuda, jit, float

• Cell [5] returns the following error

  DeprecationError: Deprecated keyword argument argtypes. Signatures should be passed as the first positional argument.

• The notebook uses Python 2.x printing.

from cfdpython.

18_Burgers_equation

• Cell [6] returns the following error

  from JSAnimation.IPython_display import display_animation

  JSAnimation is not mentioned as a dependency. A workaround that is compatible with FuncAnimation```is to use the IPython magic %matplotlib notebookinstead of%matplotlib inline`

• Markdown titles are missing a space between pound sign (#) and titles

from cfdpython.

19_Odd_Even_Decoupling

The notebook depends on a folder with videos that is missing from the repository.

from cfdpython.

Notebooks 17, 18 and 19 were never really edited or tested -- at this point my inclination is to just delete them (or move them to a subdirectory). Especially since numbapro no longer exists and the numba stuff in general is all very out of date.

Thoughts, @labarba ?

from cfdpython.

@labarba -- I've pushed up individual commits addressing the comments on notebooks 1, 2, 5, 6 and 13, respectively, in #46 . If you disagree with any of the fixes just let me know and I can pull out the individual commits in question.

I like the suggestion about using streamplot (which, if I recall, was less reliable back when we wrote these).

Here's the existing quiver plot:

And here's the same plot using streamplot instead.

Preference?

from cfdpython.

I don't know ... I like that the quiver plot shows magnitude of velocity with the arrow length.

from cfdpython.

Maybe include them both? 🤔

from cfdpython.

Regarding notebooks 17, 18, 19 — these are drafts, really… we could move them to a WiP folder?

[UPDATE] No, I think we should remove these notebooks from this repo. They could be the beginning of a new module, but "CFD Python" is round and neat up to lesson 12.

from cfdpython.

I removed notebooks 17-19 and also the very out of date Numba notebook (14). And added an additional plot to step 11 with the streamplot after the first two quiver plots. All of this in #46

from cfdpython.