Match Seurat's violin: nrd0 bandwidth, trim, and points - #89
Merged
Conversation
Three defects in vln_plot, all checked against R 4.6.1 and Seurat 5.5.1 running locally rather than against recollection. The bandwidth was 2.3x too wide. The density used scipy's "scott", which scales the sample standard deviation. R's stats::density — and so geom_violin, and so Seurat — uses bw.nrd0, which takes min(sd, IQR/1.34). Expression is zero-inflated, so the IQR term is much the smaller of the two, and on zero-inflated draws scipy came out 2.0-2.5x wider. That over-smooths away the spike at zero, which is the shape of the distribution. This was the real defect; the below-zero tails I set out to fix were the smaller half. bw.nrd0 divides by 1.34. The neighbouring rule bw.nrd, which R also ships, uses 1.349, and the first version of this used that — a silent 0.67% error wherever the IQR term wins. Diffing against R caught it. _bw_nrd0 now reproduces R to 5e-13 across seven cases, including both branches of the zero-spread fallback chain. The violin was not trimmed. geom_violin(trim = TRUE) limits the density to the observed range; an untrimmed gaussian KDE runs past it, so every violin tailed off below zero. Points were off. pt_size defaulted to 0, but Seurat's VlnPlot passes pt.size = NULL and ExIPlot resolves it through AutoPointSize, min(1583/n, 1). Points are now on by default and shrink with cell count; pt_size=0 still omits them. The outline now matches geom_violin(scale = "width", trim = TRUE)'s own computed polygon to 0.2% of full width, support equal to the data range exactly. One test earns its place specifically. Asserting _bw_nrd0 matches R says nothing about whether vln_plot calls it: substituting bw_method="scott" left all seventeen other tests green. The added test compares the drawn outline against both candidate densities and requires the nrd0 one, with 590x separation between them. Nine figures regenerated — the nine from the six generators that call vln_plot, and no others. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects in
vln_plot. All checked against R 4.6.1 / Seurat 5.5.1running locally, not against recollection of what Seurat does.
The bandwidth was 2.3x too wide
This is the real defect, and it is not the one I set out to fix.
The density used scipy's
"scott"rule, which scales the sample standarddeviation. R's
stats::density— and sogeom_violin, and so Seurat — usesbw.nrd0, which takesmin(sd, IQR/1.34). Expression is zero-inflated, so theIQR term is much the smaller of the two:
nrd0scottOver-smoothing by that much flattens the spike at zero, which is the shape of
single-cell expression.
_bw_nrd0now reproduces R to 5e-13 across sevencases, including both branches of the zero-spread fallback chain.
The violin was not trimmed
geom_violin(trim = TRUE)limits the density to the observed range. Anuntrimmed gaussian KDE runs past it, so every violin tailed off below zero,
where expression cannot go. This was the originally-reported issue and it is the
smaller half.
Points were off by default
pt_sizedefaulted to0. Seurat'sVlnPlotpassespt.size = NULL, andExIPlotresolves it throughAutoPointSize—min(1583/n, 1), read out of theinstalled package. Points are now on by default and shrink as the cell count
grows (0.60 at pbmc3k's 2638 cells);
pt_size=0still omits them.Validation
The drawn outline was compared against
geom_violin(scale = "width", trim = TRUE)'s own computed polygon, pulled out ofggplot_build:| group | max |Δ width| | support == data range |
|---|---|---|
| A | 1.4e-03 | yes |
| B | 6.4e-04 | yes |
| C | 2.1e-03 | yes |
0.2% of full width worst case; the residual is R's FFT-binned
density()againstscipy's exact evaluation. Trim confirmed exactly.
One test earns its place specifically
Asserting
_bw_nrd0matches R says nothing about whethervln_plotcallsit. Substituting
bw_method="scott"into the render path left all seventeenother tests in the file green — the component was tested, the integration was
not.
The added test compares the drawn outline against both candidate densities and
requires the nrd0 one, with 590x separation between them (4e-4 against
2.3e-1). Full mutation sweep, all five caught:
scottinstead ofnrd0Figures
Nine regenerated — exactly the nine drawn by the six generators that call
vln_plot, and no others. That the rest held still is also a check on #87: thelocked environment reproduces.
New:
violin_width, andjitter_seed(default0) so a committed figureredraws identically.
Still deliberately different from Seurat: the median bar.
geom_violindrawsnone; this keeps drawing one.
1041 passed, 25 skipped.🤖 Generated with Claude Code