lessons: actually use allocated buffers in Step12 lesson#91
lessons: actually use allocated buffers in Step12 lesson#91rohany wants to merge 1 commit intobarbagroup:masterfrom
Conversation
|
the variables pn, un, vn, b... are always re-initialized or reassigned within the main iteration loop or inside functions, and are not shared or referenced elsewhere in a way that would require maintaining their identity replacing direct assignments with slice assignments is not necessary here |
|
I agree that this is not necessary to maintain correctness, but it can improve performance and memory usage in some cases by avoiding unnecessary allocation and reusing allocated slices. |
| "source": [ | ||
| "def pressure_poisson_periodic(p, dx, dy):\n", | ||
| " pn = numpy.empty_like(p)\n", | ||
| " pn[:] = numpy.empty_like(p)\n", |
There was a problem hiding this comment.
pn doesn’t exist yet, so pn[:] = ... will raise an error
even if it did exist, you’d just be copying junk from a freshly allocated array into pn—no gain here
| "while udiff > .001:\n", | ||
| " un = u.copy()\n", | ||
| " vn = v.copy()\n", | ||
| " un[:] = u.copy()\n", |
There was a problem hiding this comment.
you allocate a temporary copy and then copy it again into un/vn. Double work.
Would suggest this instead (fastest without allocation):
un[...] = u # or: numpy.copyto(un, u)
vn[...] = v # or: numpy.copyto(vn, v)
| " vn[:] = v.copy()\n", | ||
| "\n", | ||
| " b = build_up_b(rho, dt, dx, dy, u, v)\n", | ||
| " b[:] = build_up_b(rho, dt, dx, dy, u, v)\n", |
There was a problem hiding this comment.
build_up_b still allocates a new array internally and returns it; then you copy that result into b. That’s an extra full-array copy every iteration
to improve the performance here you would need to refactor build_up_b to write in-place into a provided buffer, something like:
def build_up_b_out(b, rho, dt, dx, dy, u, v):
b[1:-1, 1:-1] = ...
b[1:-1, -1] = ...
b[1:-1, 0] = ...
I think that I know where you are coming from, but the current patch won't result in what you try to achieve. I left a couple comments with some suggestions, hope it helps. |
No description provided.