docs(userguide): clarify mc_dropout + likelihood combination#3117
Open
jbbqqf wants to merge 1 commit into
Open
docs(userguide): clarify mc_dropout + likelihood combination#3117jbbqqf wants to merge 1 commit into
jbbqqf wants to merge 1 commit into
Conversation
…#2105) The User Guide says "Monte Carlo Dropout can be combined with other likelihood estimation in Darts, which can be interpreted as a way to capture both epistemic and aleatoric uncertainty" but doesn't actually explain *how*. As reported in unit8co#2105, this leaves readers trying `mc_dropout=True` together with `predict_likelihood_parameters=True`, which is silently bypassed (the latter returns deterministic distribution parameters and skips the Monte Carlo sampling loop). This change spells out the supported combination explicitly: - train with the desired `likelihood`, - predict with `mc_dropout=True`, `num_samples >> 1`, `predict_likelihood_parameters=False`. It also adds a short note flagging the unsupported combination (`mc_dropout=True` + `predict_likelihood_parameters=True`) and links back to unit8co#2105 for context. No code is touched — this is purely a docs/userguide clarification. CHANGELOG entry under Unreleased > Improved. Refs: unit8co#2105 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
jakubchlapek
left a comment
There was a problem hiding this comment.
Hey @jbbqqf, thanks for this PR!
Looks good for me, small clarification changes :)
Comment on lines
13
to
+15
|
|
||
| - Clarified the `Capturing model uncertainty using Monte Carlo Dropout` section of the user guide to explain how `mc_dropout=True` interacts with the `likelihood`/`predict_likelihood_parameters` knobs, so readers know which combination produces both epistemic and aleatoric uncertainty. Closes [#2105](https://github.com/unit8co/darts/issues/2105). | ||
|
|
Collaborator
There was a problem hiding this comment.
feel free to take credit here (by ...) and refer to the PR number, not the issue
Suggested change
| - Clarified the `Capturing model uncertainty using Monte Carlo Dropout` section of the user guide to explain how `mc_dropout=True` interacts with the `likelihood`/`predict_likelihood_parameters` knobs, so readers know which combination produces both epistemic and aleatoric uncertainty. Closes [#2105](https://github.com/unit8co/darts/issues/2105). | |
| - Clarified the `Capturing model uncertainty using Monte Carlo Dropout` section of the `forecasting_overview.md` user guide to explain how `mc_dropout=True` interacts with the `likelihood`/`predict_likelihood_parameters` knobs. Closes [#3117](https://github.com/unit8co/darts/pull/3117). | |
Comment on lines
+293
to
+294
| > Note: `mc_dropout=True` and `predict_likelihood_parameters=True` should not be combined. `predict_likelihood_parameters=True` returns the *deterministic* parameters of the predicted distribution and bypasses the Monte Carlo sampling loop, so dropout has no observable effect on the output. See [issue #2105](https://github.com/unit8co/darts/issues/2105) for context. | ||
|
|
Collaborator
There was a problem hiding this comment.
small clarification
Suggested change
| > Note: `mc_dropout=True` and `predict_likelihood_parameters=True` should not be combined. `predict_likelihood_parameters=True` returns the *deterministic* parameters of the predicted distribution and bypasses the Monte Carlo sampling loop, so dropout has no observable effect on the output. See [issue #2105](https://github.com/unit8co/darts/issues/2105) for context. | |
| > Note: `mc_dropout=True` and `predict_likelihood_parameters=True` should not be combined. `predict_likelihood_parameters=True` is designed for use with `num_samples=1` to get the model's best estimate of the distribution parameters. | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2105.
Summary
The User Guide section Capturing model uncertainty using Monte Carlo Dropout says
…but it never says how. As @SaltedfishLZX reported in #2105, this leaves users trying
mc_dropout=Truetogether withpredict_likelihood_parameters=Trueand getting a single deterministic output, with no warning that the two flags shouldn't be combined.This PR adds three things to that section:
likelihood=..., predict withmc_dropout=True,num_samples >> 1,predict_likelihood_parameters=False.num_samplespredictions reflect both sources of uncertainty).> Noteflagging thatmc_dropout=True+predict_likelihood_parameters=Trueshould not be combined, with a link back to mc_dropout with predict_likelihood_parameters #2105 for context.No runtime code is touched — only
docs/userguide/forecasting_overview.mdand a CHANGELOG entry.Other Information
predict()indarts/models/forecasting/torch_forecasting_model.pyalready statesnum_samples = 1is required whenpredict_likelihood_parameters=True, so users do get a runtime error for the unsupported combination — no behaviour change is needed.Unreleased>Improved.Reproduce BEFORE/AFTER yourself (copy-paste)
The
sedline is identical between BEFORE and AFTER; only the checked-out ref differs.What I ran locally
darts/models/forecasting/torch_forecasting_model.py:1734-1736to confirm the docstring already enforcesnum_samples = 1forpredict_likelihood_parameters=True, so the new note in the guide is consistent with current behavior.darts/tests/exercises markdown.Edge cases tested
mc_dropout=True+predict_likelihood_parameters=Truemc_dropout=Truewith deterministic lossRisk / blast radius
Documentation-only. No code, no APIs, no tests touched. Worst-case impact is a Sphinx build that picks up the new bullet list / blockquote — both standard Markdown.
PR drafted with assistance from Claude Code (Anthropic). The clarification was reviewed manually against the discussion in #2105 and the docstring of
TorchForecastingModel.predict(). The reviewer can paste the BEFORE/AFTER block to verify.