-
Notifications
You must be signed in to change notification settings - Fork 1
[FIX] fh range should begin at 1; dependencies #1
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: main
Are you sure you want to change the base?
Conversation
fix: if the range starts in 0 it will include the last point of the training set
I added `uv` as an alternative for dependency management. The dependencies from requirements.txt were incomplete. The whole list can be found in the .toml.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
Thanks for the PR. I have a small suggestion to the code. The only major comment is could you try to rerun notebook 3. It doesn't run completely.
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 execute this notebook again and check that Moirai_small runs. The following cell seems to fail
results = benchmark.run("energy_benchmarking.json", force_rerun =True)
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.
I am unsure, do you think it is useful to push the result file?
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.
Probably it is not very useful, we can remove it.
Co-authored-by: Benedikt Heidrich <[email protected]>
Regarding the lock file, I found this discussion astral-sh/uv#10730 but I agree with you that we don't need to include it. |
Btw @benHeid testing TimeLLMForecast fails with the example of notebook 3. Code to add to replicate the issue:
|
…ause of high computing time
@benHeid ready for final review. All comments addressed, thank you. |
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.
Nice, thanks!
(@benHeid is currently off sktime
, so I am reviewing)
Could you kindly do a clean re-execution of the notebooks? I.e., reset and execute all of them.
Hi @fkiraly in theory the last PR includes a full re-run of all cells. Isn't that the case? Thanks. |
I think you need to reset the notebook and then re-execute - if the first cell shows execution number 82 and not 1, it has not been reset properly |
What does this implement/fix?