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

Fix encoder transformer and remove absolute index encoder #1257

Merged
merged 7 commits into from
Oct 4, 2022

Conversation

dennisbader
Copy link
Collaborator

Summary

Fixes the following issues:

  • encoder transformer failed when only using a cyclic encoder
  • encoder transformer was fit on all series encodings from fit(). This lead to bad results when the series used at predict() do not represent the same series. Now the transformer is globally fit on all observed values per encoded component
  • Removed absolute index position encoder and only keep relative index position encoder. Relative basically carries the same information as absolute just that it uses the end of the target series as the reference point. This also makes more sense for transformations, as the maximum value is always 0 when fitting. Users can always implement their own absolute idx logic through the „custom“ encoder (CallableIndexEncoder)

@dennisbader dennisbader requested a review from hrzn as a code owner October 4, 2022 16:45
Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

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

LGTM, nice job @dennisbader !

@hrzn hrzn merged commit 4f26eec into master Oct 4, 2022
@madtoinou madtoinou deleted the fix/encoder_transformer_cyclic_only branch July 5, 2023 21:55
@BlackFireAlex
Copy link

Relative doesn't necessarily carry the same info as absolute gives you the lifespan. Am I wrong ?

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