Skip to content

Conversation

@timhoffm
Copy link
Contributor

@timhoffm timhoffm commented Aug 28, 2021

See #19561 (comment).

I've also changed np.arange(-5, 5, 0.1) to np.linspace(-5, 5, 101) in the example because arange for non-integer steps is generally not recommended.

https://numpy.org/doc/stable/reference/generated/numpy.arange.html:

When using a non-integer step, such as 0.1, the results will often not be consistent. It is better to use numpy.linspace for these cases.

@timhoffm timhoffm changed the title DOC: Add explanation of a spare mesh grid DOC: Add explanation of a sparse mesh grid Aug 29, 2021
@charris
Copy link
Member

charris commented Sep 3, 2021

@timhoffm @rossbar Ping.

@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch 3 times, most recently from c24ec93 to 6dc3342 Compare September 3, 2021 19:38
can use the parameter ``sparse=True`` to save memory and computation time.
Here, the sparse coordinate grid reduces the memory from 2*101*101 = 20.402
to 101+101 = 202 values. Likewise, the expression ``xx**2 + yy**2`` is
reduced from 20.401 square operations to only 202 square operations, but
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct. There's both the squaring and addition operations to consider: doesn't the addition require 101**2 operations?

In any case, my two cents is that this could benefit from less detail as it's easy to lose the forest through the trees here. I'd advocate for chopping out the text from Here,: IMO the best way to provide more detail would be to add a reference to the 2011 NumPy paper, which has an excellent detailed example of broadcasting with "sparse" arrays.

Just my two cents though - leaving the detail is also fine, but we should double check to make sure the numbers are correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was only referring to the square operations. I anticipated that the square operation was more expensive than addition, but that doesn't seem to be the case. So the overall gain is theoretically just a factor 3 (beacuse the sparse sqares can be neglected compared to the addition). A bit more testing also shows that a real gain only appears when x, y have order of 1000 elements or larger, which I find surprisingly high.

grafik

Anyway, it's not within my scope to discuss performance characteristics. I've removed everything from "Here" as suggested.

@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch from 205ea53 to 994b77a Compare September 4, 2021 21:53
@charris
Copy link
Member

charris commented Sep 6, 2021

close/reopen

@charris charris closed this Sep 6, 2021
@charris charris reopened this Sep 6, 2021
Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Oops - I only meant that the text about performance should be removed, not the example. I think the clearest way to demonstrate what the sparse kwarg does is to show it:

>>> xx, yy = np.meshgrid(x, y)
>>> xx.shape, yy.shape
((101, 101), (101, 101))
>>> xs, ys = np.meshgrid(x, y, sparse=True)
>>> xs.shape, ys.shape
((1, 101), (101, 1))
>>> zs = np.sin(xs**2 + ys**2) / (xs**2 + ys**2)
>>> np.array_equal(z, zs, equal_nan=True)
True
>>> zs.shape
(101, 101)

Also, it seems that sparse keeps getting replaced with spare in the pushes.

@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch 2 times, most recently from 407ec5e to 5cb82fd Compare September 9, 2021 21:29
@timhoffm
Copy link
Contributor Author

timhoffm commented Sep 9, 2021

@rossbar Thanks, I've taken your example suggestion and adapted:

  • have separate sections for full and sparse arrays. - This example shows how to use a meshgrid to evaluate a function. Having first a complete "full" example is serves this primary purpose without already mixing in spare logic.
  • simplified the function to evaluate.
  • removed the plot. - While it is nice for illustration of "evaluate a function", the plotting code and the need for a more complex function would make the example longer and harder to read.

@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch 2 times, most recently from 8555e5c to 20e3fc4 Compare September 11, 2021 18:57
@rossbar
Copy link
Contributor

rossbar commented Sep 12, 2021

removed the plot. - While it is nice for illustration of "evaluate a function", the plotting code and the need for a more complex function would make the example longer and harder to read.

Personally I'm -1 on this change. This is an example of a very common use-case that demonstrates how these two fundamental packages (numpy and matplotlib) are used together. It'd be a shame to lose that, and I don't really think the 4 extra lines for matplotlib make the example difficult to follow.

If you feel strongly about removing it, I'd recommend opening a separate PR for it.

@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch 2 times, most recently from 2ee62f2 to c3dd580 Compare September 12, 2021 18:50
@timhoffm timhoffm force-pushed the explain-spare-meshgrid branch from c3dd580 to a151314 Compare September 12, 2021 18:52
@timhoffm
Copy link
Contributor Author

I've reverted the plot removal, Instead I've moved it to a separate block and added a colorbar.

@rossbar rossbar merged commit 8f0f33b into numpy:main Sep 22, 2021
@rossbar
Copy link
Contributor

rossbar commented Sep 22, 2021

Thanks @timhoffm !

@timhoffm timhoffm deleted the explain-spare-meshgrid branch September 22, 2021 01:19
BvB93 pushed a commit to BvB93/numpy that referenced this pull request Sep 23, 2021
* Add explanation of a spare mesh grid.

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
howjmay pushed a commit to howjmay/numpy that referenced this pull request Sep 29, 2021
* Add explanation of a spare mesh grid.

Co-authored-by: Ross Barnowski <rossbar@berkeley.edu>
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.

3 participants