-
Notifications
You must be signed in to change notification settings - Fork 951
#1103 Fixed small issues with N-BEATS and N-HITS #1130
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
…iso self.num_stacks. Also adjusted the API reference docs
Codecov Report
@@ Coverage Diff @@
## master #1130 +/- ##
==========================================
- Coverage 93.75% 93.75% -0.01%
==========================================
Files 80 80
Lines 8157 8144 -13
==========================================
- Hits 7648 7635 -13
Misses 509 509
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, only a couple of small things to fix
CHANGELOG.md
Outdated
@@ -34,6 +34,7 @@ Darts is still in an early development phase and we cannot always guarantee back | |||
- An issue with arguments being reverted for the `metric` function of gridsearch and backtest [#989](https://github.com/unit8co/darts/pull/989) by [Clara Grotehans](https://github.com/ClaraGrthns). | |||
- An error checking whether `fit()` has been called in global models [#944](https://github.com/unit8co/darts/pull/944) by [Julien Herzen](https://github.com/hrzn). | |||
- An error in Gaussian Process filter happening with newer versions of sklearn [#963](https://github.com/unit8co/darts/pull/963) by [Julien Herzen](https://github.com/hrzn). | |||
- An issue where num_stacks is used instead of self.num_stacks in the NBEATSModel. Also, a few mistakes in API reference docs [#1103](https://github.com/unit8co/darts/pull/1103) by [Rijk van der Meulen](https://github.com/rijkvandermeulen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for adding an entry, however you should add it under the Unreleased
section (not the already-release 0.20.0 version).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad, of course. I've changed it.
@@ -760,7 +759,7 @@ def __init__( | |||
self.num_stacks = 2 | |||
|
|||
if isinstance(layer_widths, int): | |||
self.layer_widths = [layer_widths] * num_stacks | |||
self.layer_widths = [layer_widths] * self.num_stacks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also bring the same fix to nhits.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -574,7 +574,6 @@ def __init__( | |||
The number of blocks making up every stack. | |||
num_layers | |||
The number of fully connected layers preceding the final forking layers in each block of every stack. | |||
Only used if `generic_architecture` is set to `True`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also change the docstring of _NBEATSModule
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes an issue where num_stacks is used instead of self.num_stacks in the NBEATSModel. Also addresses a few mistakes in API reference docs.
Summary
Other Information
Fixed an unrelated minor typo somewhere else in the docs