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

Fit antialiased line code within usual numba/dask framework #1142

Merged
merged 16 commits into from
Nov 3, 2022
Merged

Fit antialiased line code within usual numba/dask framework #1142

merged 16 commits into from
Nov 3, 2022

Conversation

ianthomas23
Copy link
Member

This PR modifies both the antialiased line implementation and the low level numba/dask framework code so that the former now works within the latter rather than having its own specific code path. This is necessary before:

  1. Support for compound Reductions with antialiased lines (issue Antialiasing fails on uint32 with a mean reduction #1133)
  2. Antialiased lines on the GPU. This is part-implemented herein, but it turns out that compound Reduction support is needed first.

There is no functional change from the end user's point of view.

Catalogue of changes:

  1. New antialiased kwarg for compile_components(), the various make_* functions, and the pipeline functions. The latter now have two optional bool args antialias and cuda so I have forced them to be kwargs to avoid potential ambiguity.
  2. Glyph class stores a bool antialias attribute. This is only used within _expand_aggs_and_cols which is the function called when we use the @expand_aggs_and_cols decorator.
  3. antialias flag passes through the various Line classes and line rendering functions in line.py.
  4. _full_antialias now works with the @expand_aggs_and_cols decorator which is necessary for it to work on the GPU.
  5. _full_antialias takes a workspace which is an array (numpy or cupy) of length 8. CUDA cannot create arrays so it has to be passed in from outside the function, this should eventually (after other changes) give increased performance for all antialiased lines due to many fewer array allocations and deallocations.
  6. Remove the any_f32 and count_f32 reductions completely.
  7. Reduction classes modified so that their dtypes are not static. Now the same reduction can be used for antialiased and non-antialiased lines, and it will create the appropriately dtyped aggs and use the correct append functions. There are some refactors here that can be done in the future to reduce the duplication of the various append and create functions, but this PR is large enough as it is.

@ianthomas23 ianthomas23 added this to the v0.14.3 milestone Nov 1, 2022
@ianthomas23
Copy link
Member Author

ianthomas23 commented Nov 3, 2022

Rebased to include #1143 to avoid unrelated CI failures.

@jbednar
Copy link
Member

jbednar commented Nov 3, 2022

Ian and I went over this PR in a shared meeting, and I'm ok with these changes after a trivial suggestion to rename "workspace" to something like "buffer" so that it's clearer that it's simply a block of memory for the callee to use as it likes. Thanks, @ianthomas23 !

@ianthomas23
Copy link
Member Author

workspace renamed to buffer, and added some more explanation as comments in the code.

For clarity, a buffer is used entirely within the lifetime of a _full_antialias call, the values are set once and then read multiple times. It doesn't matter what the buffer contains before or after _full_antialias is called. buffers are created at a higher level so that there is one per CPU/GPU thread to avoid cross-talk.

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #1142 (c3b7fc4) into master (cf77868) will increase coverage by 0.16%.
The diff coverage is 88.91%.

@@            Coverage Diff             @@
##           master    #1142      +/-   ##
==========================================
+ Coverage   84.99%   85.16%   +0.16%     
==========================================
  Files          33       33              
  Lines        7519     7632     +113     
==========================================
+ Hits         6391     6500     +109     
- Misses       1128     1132       +4     
Impacted Files Coverage Δ
datashader/data_libraries/cudf.py 57.14% <0.00%> (ø)
datashader/data_libraries/dask_cudf.py 57.14% <0.00%> (ø)
datashader/reductions.py 85.35% <83.67%> (+0.68%) ⬆️
datashader/glyphs/line.py 93.21% <95.27%> (-0.25%) ⬇️
datashader/compiler.py 96.15% <100.00%> (+0.15%) ⬆️
datashader/core.py 87.99% <100.00%> (ø)
datashader/data_libraries/dask.py 94.49% <100.00%> (ø)
datashader/data_libraries/dask_xarray.py 98.91% <100.00%> (ø)
datashader/data_libraries/pandas.py 100.00% <100.00%> (ø)
datashader/data_libraries/xarray.py 100.00% <100.00%> (ø)
... and 3 more

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

@ianthomas23 ianthomas23 merged commit e888ba3 into holoviz:master Nov 3, 2022
@ianthomas23 ianthomas23 deleted the antialiasing_framework branch November 3, 2022 18:09
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

Successfully merging this pull request may close these issues.

2 participants