-
Notifications
You must be signed in to change notification settings - Fork 5
Updated variable types, documentation and names #45
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
base: master
Are you sure you want to change the base?
Updated variable types, documentation and names #45
Conversation
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.
Just a few comments.
virtual | ||
void | ||
consume (InputType sample, | ||
AuxiliaryData aux_data) override; |
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.
Can you run the indentation script? I think that is necessary for these lines.
/** | ||
* A function that returns the average cosine vector computed from the | ||
* samples seen so far. If no samples have been processed so far, then | ||
* a default-constructed object of value_type will be returned. |
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.
Write this as follows to make sure we get nice markup when doxygen converts the documentation to HTML:
* a default-constructed object of value_type will be returned. | |
* a default-constructed object of `value_type` will be returned. |
* definition of "autocovariance" is more complex than what we do here (except | ||
* if we work scalar samples types). That said, below we will use the word | ||
* "autocovariance" even when refering to this spurious autocovariance. | ||
* "autocovariance" even when refering to this spurious autocovariance. It can be looked as trace of |
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.
* "autocovariance" even when refering to this spurious autocovariance. It can be looked as trace of | |
* "autocovariance" even when refering to this spurious autocovariance. It can be looked as the trace of |
* @f} | ||
* In other words, it calculates the covariance of samples $x_{t+l}$ and $x_t$ | ||
* with a lag between zero and $k$ | ||
* with a lag l between zero and $k$ |
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.
* with a lag l between zero and $k$ | |
* with a lag $l$ between zero and $k$ |
* \hat\gamma(l) | ||
* = | ||
* \frac{1}{n} \sum_{t=1}^{n-l}{(x_{t+l}-\bar{x})(x_{t}-\bar{x})}. | ||
* \frac{1}{n-l} \sum_{t=1}^{n-l}{(x_{t+l}-\bar{x})(x_{t}-\bar{x})}. |
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.
OK :-)
Any news? |
As discussed on Friday, I have updated code for average cosinus. Also, I have fixed mistakes at the documentation of spurious autoregression and changed the variable name from "current_autocovariation" to "current_spur_autocovariance". All tests passed successfully.