-
Notifications
You must be signed in to change notification settings - Fork 36
Implement confidence intervals for predictions #104
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
3479ce7 to
32fcc1f
Compare
|
The new implementation stores the rows of the hat matrix (and a bit more) for each vertex when constructing the Loess fit. That requires much more memory than the old implementation and this causes the benchmark runs to run out of memory because it tests relatively large problems, see Loess.jl/benchmark/benchmarks.jl Lines 7 to 12 in 5ecfefa
loess function and only constructs when when you call predict on the loess object but you pay the price of essentially recomputing all the local fits in predict. In my opinion, the main use of Loess these days is for visualization with uncertainty bounds, so I'm leaning towards just accepting that the implementation can't handle as large problems.
|
Benchmark Report for /home/runner/work/Loess.jl/Loess.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTargetBaseline |
bd5ef22 to
a8c2129
Compare
|
I've reduced the problem size of the benchmarks to not exceeding 10000 elements, and that goes through without hitting the memory limit. The overall runtime is slower, which shouldn't be a surprise, given that more work is being done and more allocations are required to be able to compute the uncertainty of the predictions. The question is if there is a need for very large er very quick Loess computations where the uncertainty calculation is just a nuisance. |
a8c2129 to
e8840fd
Compare
Sorry for the delay in following up here, but this is exactly my biggest concern. I have a use case where we put a smooth line on a plot with a very large number of points, but I haven't had a chance to test the performance here on that. I also sometimes use a smooth line on plots of Bernoulli data -- the smooth gives a good first impression of how the probability changes across different x values. I'll try to find some time to test on these big examples this week! |
|
@palday It would indeed be good if you could try out some of your larger examples. If we conclude that supporting large problems without uncertainty is important, I can see a couple of ways forward. I'm not particularly happy with any of them so my preference is for the current version, but I'm of course biased by my use cases. The alternatives I see are
|
9a2d0f3 to
730608d
Compare
|
@devmotion thanks for the comments. I believe I've addressed all of them now. |
|
@palday did you get a chance to run your examples? I'm quite eager to get error bounds in Loess plots, so it would be good to find a path forward here. |
|
@palday bump. Do you object to this or are you okay with moving forward here? |
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
And some other cleanup after merging review comments
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
Test on min and lts
Also, use norm instead of sqrt(sum(abs2,...))
Co-authored-by: David Müller-Widmann <devmotion@users.noreply.github.com>
8f03018 to
8f5f6ba
Compare
|
@devmotion it looks like the benchmarks now complete but then fail afterwards with an error about the repo being dirty. Have you seen that error before? |
2e04089 to
8f5f6ba
Compare
|
No, I have never seen this error. I wonder if it could be related to either the GHA update or the 1.12.2 release. |
|
Ah, I think it could be JuliaCI/PkgBenchmark.jl#135 which was caused by a Project.toml file that Pkg automatically reformatted, and hence the repo seemed to be dirty in CI. Edit: Although the Project.toml seems fine to me... |
|
Generally, maybe the benchmark setup would be a bit simpler with AirspeedVelocity.jl. But of course either approach should be fine. |
ae37d76 to
338a264
Compare
|
@devmotion your intuition was right here. With Julia 1.12.2, the |
338a264 to
1a08272
Compare
1a08272 to
835d6b6
Compare
|
cc @bjarthur as this might be useful in Gadfly. |
|
Gadfly needs a new maintainer. has for a while. i have switched to Makie going forward. |
This implements the confidence interval as described in Cleveland and Grosse 1991 except for the statistical approximation of the deltas described in section 4 of the paper. They don't seem to share the coefficients of their fit, so it is not easy to implement that part. In addition, computers have much more memory these days, so I don't think the big matrix is a problem in most cases. I'd be interested in anybody knows about more recent approaches to calculating the deltas without the need for the big matrix.
I tried to mimic the interface for predict in
GLMbut decided to use a struct to return the expensive helper quantities together with the confidence bounds. I'm not a big fan of usingSymbols for finite options like here but that is whatGLMcurrently does. Maybe we should change it, but that is a separate concern.With this, you can construct this plot
The same plot with
ggplot2which uses Cleveland and Grosses code for the computations isCloses #29