-
-
Notifications
You must be signed in to change notification settings - Fork 372
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
Fit antialiased line code within usual numba/dask framework #1142
Conversation
Rebased to include #1143 to avoid unrelated CI failures. |
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 ! |
For clarity, a |
Codecov Report
@@ 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
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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:
Reduction
s with antialiased lines (issue Antialiasing fails on uint32 with a mean reduction #1133)Reduction
support is needed first.There is no functional change from the end user's point of view.
Catalogue of changes:
antialiased
kwarg forcompile_components()
, the variousmake_*
functions, and thepipeline
functions. The latter now have two optional bool argsantialias
andcuda
so I have forced them to be kwargs to avoid potential ambiguity.Glyph
class stores a boolantialias
attribute. This is only used within_expand_aggs_and_cols
which is the function called when we use the@expand_aggs_and_cols
decorator.antialias
flag passes through the variousLine
classes and line rendering functions inline.py
._full_antialias
now works with the@expand_aggs_and_cols
decorator which is necessary for it to work on the GPU._full_antialias
takes aworkspace
which is an array (numpy
orcupy
) 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.any_f32
andcount_f32
reductions completely.append
functions. There are some refactors here that can be done in the future to reduce the duplication of the variousappend
andcreate
functions, but this PR is large enough as it is.