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

FEAT: Support exog features in TimeMixer #1088

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

marcopeix
Copy link
Contributor

Fix bug that allows TimeMixer to take futr_exog now.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@marcopeix marcopeix added the fix label Jul 29, 2024
@JQGoh
Copy link
Contributor

JQGoh commented Jul 30, 2024

@marcopeix any chance this could be part of 1.7.4 release (understood that 1.7.4 has just been released). Sounds like this is a major issue that prevents a key part of TimeMixer working properly.

@elephaint
Copy link
Contributor

elephaint commented Jul 30, 2024

@JQGoh 1.7.4 was already released today, we took out the futr_exogenous functionality of TimeMixer for now, but we can make it part of the next release.

@JQGoh
Copy link
Contributor

JQGoh commented Jul 30, 2024

@JQGoh 1.7.4 was already released today, we took out the futr_exogenous functionality of TimeMixer for now, but we can make it part of the next release.

Thanks for the info. I was not aware that the futr_exogenous support was removed.

@elephaint
Copy link
Contributor

@JQGoh 1.7.4 was already released today, we took out the futr_exogenous functionality of TimeMixer for now, but we can make it part of the next release.

Thanks for the info. I was not aware that the futr_exogenous support was removed.

Yeah @marcopeix and I couldn't make it work easily and we didn't understand it sufficiently, and we wanted to move forward with the release...

@peacocktrain
Copy link

@marcopeix Hello, I would like to use the futr_exog feature of TimeMixer, and I have pulled down this branch to test it. However, the content provided in the ipynb file does not execute correctly, and the following error is reported:

File "/workspace/neuralforecast/common/_modules.py", line 388, in forward x = self.tokenConv(x.permute(0, 2, 1)).transpose(1, 2) RuntimeError: permute(sparse_coo): number of dimensions in the tensor input does not match the length of the desired ordering of dimensions i.e. input.dim() = 4 is not equal to len(dims) = 3

Could you please help me check if the modifications in the ipynb file can be executed correctly? Thank you.

@elephaint
Copy link
Contributor

elephaint commented Sep 4, 2024

@peacocktrain This is a draft PR, which means it doesn't necessarily work. Please don't use draft PRs to work with, as functionality likely doesn't work properly.

For now, I'd advise to use another model that supports futr_exog while we work on enabling this feature for TimeMixer.

@marcopeix marcopeix changed the title [BUGFIX] - Fix bug for futr exog in TimeMixer FEAT: Support exog features in TimeMixer Sep 4, 2024
@marcopeix
Copy link
Contributor Author

Supporting exogenous features in TimeMixer is a feature that is on our radar. Note that the model as presented in the original paper does not use exogenous features. So, in the current state, the model in neuralforecast can be used to reproduce the results of the paper.

@marcopeix marcopeix linked an issue Sep 4, 2024 that may be closed by this pull request
@elephaint
Copy link
Contributor

The original paper doesn't seem to support exogenous features. Upon learning this I don't see a reason to support this.

Note that it's not easy to include exogenous - it is insanely hard to do this right, so it seems a limitation of the architecture that we are not placed to solve.

I'd vote for closing unless I'm misunderstanding here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NeuralForecast |TimeMixer Model - exogeneous variables support
4 participants