holoviz / datashader

Quickly and accurately render even the largest data.
http://datashader.org
BSD 3-Clause "New" or "Revised" License
3.3k stars 365 forks source link

Fix conversion from cupy in categorical rescale_discrete_levels #1179

Closed ianthomas23 closed 1 year ago

ianthomas23 commented 1 year ago

See issue #1178.

When using cudf, a categorical aggregate, how='eq_hist' and rescale_discrete_levels=True then categorical shading raises an error when trying to convert a tuple that contains a cupy array and scalar to a cupy array.

The simplest possible fix would have been in _rescale_discrete_levels() here: https://github.com/holoviz/datashader/blob/d8167b41577058abe01ae1b847a5178ae6ccf172/datashader/transfer_functions/__init__.py#L230-L232 by casting lower_span to a float. This is fine if lower_span ia a scalar or numpy array, but if it is a cupy array then this would transfer it from GPU to CPU memory, only to be pushed back to the GPU subsequently. The solution here is to convert the span from tuple to numpy/cupy array using hstack in _interpolate_alpha.

This fixes the error produced by the OP's example code, but there is still a problem in holoviews not displaying the categorical legend if using the GPU. I will create a separate holoviews issue for this.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1179 (efb55d0) into main (d8167b4) will increase coverage by 0.00%. The diff coverage is 75.00%.

@@           Coverage Diff           @@
##             main    #1179   +/-   ##
=======================================
  Coverage   85.38%   85.38%           
=======================================
  Files          35       35           
  Lines        8011     8012    +1     
=======================================
+ Hits         6840     6841    +1     
  Misses       1171     1171           
Impacted Files Coverage Δ
datashader/transfer_functions/__init__.py 86.91% <75.00%> (+0.02%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more