-
Notifications
You must be signed in to change notification settings - Fork 39
make sure threadsafe rasterization ops are efficient #1039
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
Conversation
src/methods/rasterize.jl
Outdated
| _is_op_threadsafe(::typeof(maximum)) = true | ||
| # Identical to Base.PermutedDimsArrays.CommutativeOps but define here to avoid | ||
| # using base internals | ||
| const COMMUTATIVE_OPS = Union{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const COMMUTATIVE_OPS = Union{ | |
| const CommutativeOps = Union{ |
Actually a type not a constant
src/methods/rasterize.jl
Outdated
| typeof(*) | ||
| } | ||
|
|
||
| _is_op_threadsafe(::COMMUTATIVE_OPS) = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| _is_op_threadsafe(::COMMUTATIVE_OPS) = true | |
| _is_op_threadsafe(::CommutativeOps) = true |
|
I really have no clue what is going on in the Documentation build. Seems to be some git issue? |
|
No, there is some problem in an example block, but it doesn't tell me which example block for some reason. Very weird - normally it does. |
|
it is a link (asset) that is not downloading. We definitely need an independent source for these kind of assets. Basically, any USA based related link is a gamble. |
|
Got a link to where that is? I can only see logging related errors |
|
ahh no, sorry this is the issue ( ? is a Makie issue) @asinghvi17 ? |
|
I thought we commented that out? |
|
But that can just get deleted probably and maybe if someone can file an issue about that, I can get around to looking at it soonish |
|
Ah if it's still that issue I made a PR to makie to fix it: MakieOrg/Makie.jl#5327 |
|
I think we can merge this? The documentation issue will be fixed whenever the Makie folks bump ComputePipeline |
closes #1032