-
Notifications
You must be signed in to change notification settings - Fork 265
Adds support for passing initial value to transform #2038
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
Adds support for passing initial value to transform #2038
Conversation
Thank you for the contribution, @tingiskhan! Do you think this could also be achieved using a composition of an |
That’s a very good point, I’ll take a look at it next week (if you don’t beat me to it)! |
@tillahoffmann So, I've given it some thought and see three ways forward (there might be more):
Of these three I think either 1. or 3. is the most appropriate. I'm not sure how re-usable the new component of 3. would be, but I'm open to either suggestions. I'm not sure this PR is even needed to be honest, but the current way of passing initial values other than 0 is not so clear (might just require an entry to the docs?). |
If initial value is 0, we have y_1=x_1, so we can just prepend the initial value to x and y? |
@fehiepsi Yes, I suppose that would work as well! What solution would be preferred (if at all)? |
Hi @tingiskhan, I think supporting initial values makes sense. |
@fehiepsi, are these changes what you had in mind? |
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.
LGTM pending some nits. Thanks @tingiskhan!
@fehiepsi, I’m guessing the errors are unrelated to my changes? |
Could you help update the tolerant of the failing tests? |
…nsform together with parameterization
@fehiepsi, the tests were failing because of the |
@fehiepsi, equinox released a new version yesterday. Pinned it (sort of) and adressed the exception by passing the |
Changes made
Added support for passing a parameter called
initial_value
toRecursiveLinearTransform
to facilitate forecasting when using the transform for latent variables.Links to related PRs
#2037
Tests
I added one test case to the "general" part, and parameterized
test_batched_recursive_linear_transform
for testing arbitrary initial values.Dependencies
No new dependencies introduced.