Skip to content

Conversation

@apointa
Copy link
Contributor

@apointa apointa commented May 27, 2025

Note: This PR starts working only after https://github.com/NX-AI/tirex is published

Description of changes:
Add TiRex as a new model.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@shchur shchur left a comment

Choose a reason for hiding this comment

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

Just a few minor comments, otherwise LGTM. The API of the ForecastModel class looks pretty clean, so the wrapper is quite easy to understand.

- pip:
- --index-url https://download.pytorch.org/whl/cu126
- --extra-index-url https://pypi.org/simple
- torch~=2.6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the requirements.txt and requirements_py26.yaml define slightly different version ranges. Are both of these files supposed to be installed into the same environment, or do they provide two alternative options for installing the dependencies?

I would recommend to define

The naming requirements_py26.yaml is also a bit strange as it seems to imply that these a requirements for Python 2.6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So in general it should work just installing the requirements.txt, however, we recommend installing TiRex inside the provided conda environment as we observe some people have problems with CUDA stuff otherwise. (TiRex uses custom CUDA kernels of the sLSTM)

I relaxed the major dependencies lock of the requirements.txt, and added a conda export of the environment on which i ran the test (freezed_test_environment.yaml)

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, it's okay to only keep requirements.txt and the frozen_test_environment.yaml. I would recommend pointing to the official installation instructions in the TiRex repo (the corresponding environment.yaml). This way neither you nor we will need to worry if the current requirements.yaml file stored in the fev repo is up-to-date.

@apointa
Copy link
Contributor Author

apointa commented May 27, 2025

updated the commits with correct commit email adress

@apointa
Copy link
Contributor Author

apointa commented May 27, 2025

the TiRex repository and the Model on HF are public so the PR should work now : )

@shchur shchur merged commit d389a2a into autogluon:main May 28, 2025
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.

2 participants