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

Poor performance of scatter with non-scalar markersize #3307

Closed
3 tasks done
pablosanjose opened this issue Oct 20, 2023 · 8 comments · Fixed by #3113
Closed
3 tasks done

Poor performance of scatter with non-scalar markersize #3307

pablosanjose opened this issue Oct 20, 2023 · 8 comments · Fixed by #3113
Labels

Comments

@pablosanjose
Copy link
Contributor

This is a performance issue. When calling scatter with a markersize that is a collection (say, a Vector) the performance of display is around ~200 times slower than with a scalar markersize. meshscatter does not have this problem, so I suspect it is a missed optimization of some kind.

Reproducer (timings and allocs are second-run)

julia> r = 10 * rand(Point3, 200000); s = rand(200000);

julia> @time display(scatter(r, markersize = 0.5));
  0.106780 seconds (58.04 k allocations: 8.244 MiB, 6.92% compilation time)

julia> @time display(scatter(r, markersize = s));
 18.634452 seconds (333.85 M allocations: 16.129 GiB, 3.90% gc time, 0.02% compilation time)

julia> @time display(meshscatter(r, markersize = s));
  0.024103 seconds (49.81 k allocations: 10.078 MiB)
  • are you running newest version (version from docs) ? Yes
  • can you reproduce the bug with a fresh environment ? (]activate --temp; add Makie) Yes
  • What platform + GPU are you on? Macbook Pro M1, macos Sonoma 14.0
@SimonDanisch
Copy link
Member

Funny that this turns up now, where I just fixed it :D This performance "bug" has been in Makie for quite a while and I literally run into it yesterday and fixed here:
#3281

@pablosanjose
Copy link
Contributor Author

Oh, wow, what a coincidence! That's so nice, thanks @SimonDanisch!

Hate to be pushy, but any change that could be merged by Monday? (It would be good for a presentation I'm giving :-))

@SimonDanisch
Copy link
Member

That's pretty impossible, since it's already end of day here and it will take a while to get those changes out of the beta release and then tag makie ...

@pablosanjose
Copy link
Contributor Author

Ok, no worries! Thanks for all your work, it makes me smile every day.

@jkrumbiegel
Copy link
Member

You could try running your demo on the beta branch? It should be mostly functional

@SimonDanisch
Copy link
Member

That, or I just realize you can also use marker=Circle to avoid this... What's slow is the BezierPath marker, which is the default.

@pablosanjose
Copy link
Contributor Author

That works!! And it looks exactly the same, I think? Wonderful!

@jkrumbiegel
Copy link
Member

The default circle and Circle have a slight size difference, as Circle has radius 1 I think and the other is adjusted to look good with the other default markers and glyphs.

@SimonDanisch SimonDanisch mentioned this issue Oct 27, 2023
SimonDanisch added a commit that referenced this issue Nov 17, 2023
Continues #2831 !
Still needs to check, if I rebased correctly and didn't incorrectly
apply some of the changes!

## Merged PRs
- #2598
- #2746
- #2346
- #2544
- #3082
- #2868
- #3062
- #3106
- #3281
- #3246

## TODOS

- [x] fix flaky test `@test GLMakie.window_size(screen.glscreen) ==
scaled(screen, (W, H))`
- [x] Merge axis type inferences from #2220 
- [x] Test on different resolution screens, IJulia, Pluto, VSCode,
Windowed
- [x] rebase to only have merge commits from the PRs 
- [x] investigate unexpected speed ups
- [x] reset camera settings from tests
- [ ] check doc image generation
- [x] rethink default near/far in Camera3D (compatability with OIT)
- [x] merge #3246
- [x] fix WGLMakie issues/tests:
- [x] fix line depth issues (see tests: ~~hexbin colorrange~~ (not new),
LaTeXStrings in Axis3, Axis3 axis reversal)
  - [x] fix lighting of surface with nan points (fixed in #3246)
- ~~volume/3D contour artifacts (see 3D Contour with 2D contour
slices)~~ not new
  - ~~artifacting in "colorscale (lines)"~~ not new
- [x] GLMakie:
  - [x] slight outline in "scatter image markers" test
  - ~~clipping/z-fighting in "volume translated"~~ not new
- [x] CairoMakie:
  -  ~~Artfiacting in `colorscale (lines)"~~ not new
  - ~~markersize in "scatter rotations" changed?~~ not new
  - ~~color change in "colorscale (poly)"~~ not new
  - ~~transparency/render order of "OldAxis + Surface"~~ not new
  - ~~render order in "Merged color mesh"~~ not new
  - ~~render order of "Surface + wireframe + contour"~~ not new
- [x] Check "SpecApi in convert_arguments" (colors swapped?)


## Fixes the following errors

- fixes #2721 via #2746
- fixes #1600 via #2746
- fixes #1236 via #2746
- fixes MakieOrg/GeoMakie.jl#133 via #2598
- closes #2522
- closes #3239 via #3246
- fixes #3238 via #3246
- fixes #2985 via #3246
- fixes #3307 via #3281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants