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

Holt-Winters seasonal method #142

Closed
wants to merge 10 commits into from
Closed

Holt-Winters seasonal method #142

wants to merge 10 commits into from

Conversation

rchanda
Copy link
Contributor

@rchanda rchanda commented Mar 29, 2016

No description provided.

@rchanda rchanda changed the title [WIP] Holt-Winters seasonal method Holt-Winters seasonal method Mar 29, 2016
@sryza
Copy link
Owner

sryza commented Apr 4, 2016

This looks awesome. Thanks a ton.

Curious to hear any details of the R comparison? The error tends to be higher than R? How much higher? Can still accept the PR if so, just want to be able to document this properly.

A few notes:

  • Line endings should be under 100 characters
  • Indentation is inconsistent in some files
  • The branch currently has merge conflicts
  • Classes and methods need documentation to describe what they do and which methods are used
  • Files need copyright headers at the top

Are you interested in fixing these things? If not, let me know, and I can work on them when I get the chance.

@dircsem
Copy link

dircsem commented May 17, 2016

Why does the forecast method receive two vectors? Isn't better to create the forecast vector inside this method?

@dircsem
Copy link

dircsem commented Jun 8, 2016

Ok, but don't you think that would be better to maintain the same structure than is in ARIMA?
I can do this change, and some others like
for (i <- 0 to (initSeason.size - 1)
to
for (i <-0 until (initSeason.size )
if I have write access to this branch.

@sryza
Copy link
Owner

sryza commented Jun 14, 2016

you are a champion. This looks great. The main nits I have are cosmetic:

  • Indentation needs to be kept at a consistent 2 spaces.
  • There needs to be a space between "if" and "for" keywords and the parentheses that follow them.
  • The patch needs to be rebased so that there are no merge conflicts.

Let me know if you're interested in making these changes. If not, I'll happily do so.

@rchanda rchanda closed this Jun 16, 2016
@rchanda rchanda deleted the holtwinters branch June 16, 2016 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants