-
Notifications
You must be signed in to change notification settings - Fork 642
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
[DOC] Correct argument for optimizer ranger
in Temporal Fusion Transformer
tutorial
#1724
base: main
Are you sure you want to change the base?
Conversation
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.
the notebook CI failed, looks like a timeout? I will restart
The issue was the training of the Also increased the timeout in the tests to 1200s due to the training time |
@@ -180,3 +180,8 @@ def setup(app: Sphinx): | |||
suppress_warnings = [ | |||
"autosummary.import_cycle", | |||
] | |||
|
|||
# -----------nbsphinx extension ---------- | |||
nbsphinx_execute = "never" # always |
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.
If you never run the notebooks, do the below the configurations have any effect?
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 found that there is a workflow run for testing the notebook and it was one causing the timeout error, this is for building the docs.
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.
If I am not wrong, nbsphinx_execute = "never"
will ensure that RTD will not execute the example notebooks while building the docs (ref. https://nbsphinx.readthedocs.io/en/0.9.5/never-execute.html). If that is the case, whether that execution need to ignore errors or not (nbsphinx_allow_errors = False
) or how long execution takes (nbsphinx_timeout = 600
) should not matter.
Does that make sense? Let me know if I am missing something, or misunedrstanding the purpose of these configurations.
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.
You're right, these extra configurations are meaningless.
should we keep the nbsphinx_execute = "never"
? as we run notebooks in this workflow to test if they don't fail.
run-notebook-tutorials: |
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 believe never
is fine, these executions increase RTD time significantly. Let's skip in sktime
too for that matter, if @fkiraly agree as well (I have seen too many timeouts recently).
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'm unable to see this diff in Github (it saus unable to render rich diff), and ReviewNB also seems to lack the notebook.
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.
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.
Not sure why it's not working for me, but not a big deal. I checked it from branch it directly.
I am not what change of yours is making the predictions worse. In main
branch, the predicted orange line and shaded area seemed to contain the blue actuals pretty well, but in the modified one the gaps seem to have increased notably. Do you agree with observation? Any guess what's causing it (seed remained same at 42 in both cases)?
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'm not sure about the cause as I can see even the learning_rate got changed from 0.041
to 0.009
. I think I'll need to check the logs and see where it is going wrong
fixes error in the tutorial which was causing examples to fail.
fixes #1722