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.

Sign up for GitHub

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

Jump to bottom

Add exponential weights #401

Merged
merged 13 commits into from
Jun 27, 2019
Merged

Add exponential weights #401

merged 13 commits into from
Jun 27, 2019

Conversation

ararslan
Copy link
Member

@ararslan ararslan commented Jul 24, 2018 •
edited
Loading

This is almost entirely code written by @rofinn. I'm just submitting it here with a couple of slight modifications.

@codecov
Copy link

codecov bot commented Jul 24, 2018 •
edited
Loading

Codecov Report

Merging #401 into master will increase coverage by 6.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   83.99%   90.07%   +6.08%     
==========================================
  Files          21       21              
  Lines        2162     2026     -136     
==========================================
+ Hits         1816     1825       +9     
+ Misses        346      201     -145
Impacted Files Coverage Δ
src/StatsBase.jl 100% <ø> (ø) ⬆️
src/weights.jl 84.44% <100%> (+7.84%) ⬆️
src/signalcorr.jl 99.11% <0%> (+0.43%) ⬆️
src/toeplitzsolvers.jl 34.69% <0%> (+0.69%) ⬆️
src/transformations.jl 93.4% <0%> (+1.01%) ⬆️
src/counts.jl 90.27% <0%> (+1.23%) ⬆️
src/cov.jl 95.91% <0%> (+3.76%) ⬆️
src/hist.jl 91.05% <0%> (+4.11%) ⬆️
src/sampling.jl 92.05% <0%> (+4.9%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aad1fe0...02e0c5e. Read the comment docs.

joshday
joshday reviewed Jul 24, 2018
View reviewed changes
docs/src/weights.md Outdated Show resolved Hide resolved
joshday
joshday reviewed Jul 24, 2018
View reviewed changes
src/weights.jl Outdated Show resolved Hide resolved
nalimilan
nalimilan reviewed Sep 2, 2018
View reviewed changes
src/weights.jl Outdated Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
test/weights.jl Outdated Show resolved Hide resolved
test/weights.jl Outdated Show resolved Hide resolved
@ararslan
Copy link
Member Author

I've pared this down to remove the automatic normalization and I've deleted the variance correction method (so exponential weights will fall back to the generic definition which produces an error), corrected the language used in the documentation, removed the eweights method that accepts a vector, and removed the tests for functionality no longer implemented.

@omus
Copy link

omus commented Feb 15, 2019

Bump

@nalimilan
Copy link
Member

Still need to address https://github.com/JuliaStats/StatsBase.jl/pull/401/files#r214542378

@nalimilan nalimilan mentioned this pull request May 2, 2019
Open
@rofinn
Copy link
Member

rofinn commented May 28, 2019

@ararslan Are you okay if I update your branch?

@ararslan
Copy link
Member Author

Yes, please do!

@rofinn @ararslan
Add exponential weights
b2c01a5 
Co-authored-by: Alex Arslan <ararslan@comcast.net>
@rofinn rofinn force-pushed the aa/temporal-weights branch 2 times, most recently from 245409f to 7482a38 Compare May 28, 2019 21:10
rofinn
rofinn reviewed May 28, 2019
View reviewed changes
src/weights.jl Show resolved Hide resolved
nalimilan
nalimilan reviewed Jun 1, 2019
View reviewed changes
docs/src/weights.md Outdated Show resolved Hide resolved
docs/src/weights.md Outdated Show resolved Hide resolved
docs/src/weights.md Outdated Show resolved Hide resolved
docs/src/weights.md Outdated Show resolved Hide resolved
docs/src/weights.md Outdated Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
src/weights.jl Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
test/weights.jl Outdated Show resolved Hide resolved
rofinn and others added 6 commits June 3, 2019 10:44
@rofinn @nalimilan
Update docs/src/weights.md
22c0f62 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update docs/src/weights.md
6fdf82d 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update docs/src/weights.md
71296bd 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update src/weights.jl
e9e2796 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update src/weights.jl
8bb948f 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn
Minor cleanup
96275d8
@rofinn
Copy link
Member

rofinn commented Jun 3, 2019 •
edited
Loading

Okay, apart from your comments around the eweights(t, r, λ::Real) I think that's everything you commented on.

nalimilan
nalimilan reviewed Jun 25, 2019
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few last remarks and I think it's good!

src/weights.jl Outdated Show resolved Hide resolved
src/weights.jl Show resolved Hide resolved
src/weights.jl Outdated Show resolved Hide resolved
rofinn and others added 2 commits June 25, 2019 10:06
@rofinn @nalimilan
Update src/weights.jl
b132deb 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update src/weights.jl
bbc3850 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn @nalimilan
Update src/weights.jl
02e0c5e 
Co-Authored-By: Milan Bouchet-Valat <nalimilan@club.fr>
@rofinn
Copy link
Member

rofinn commented Jun 26, 2019

I guess if nobody has any other comments then I'll merge it tomorrow.

@rofinn rofinn merged commit a09fe50 into JuliaStats:master Jun 27, 2019
@ararslan
Copy link
Member Author

Thanks for getting this finished up, Rory. :)

@nickrobinson251 nickrobinson251 mentioned this pull request Jul 3, 2019
Closed
@rofinn rofinn mentioned this pull request Jul 3, 2019
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rofinn rofinn rofinn left review comments

@joshday joshday joshday left review comments

@nalimilan nalimilan nalimilan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ararslan @omus @nalimilan @rofinn @joshday