-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add sharp bits section and invite users to the discussions upon error #391
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
+ Coverage 84.45% 84.70% +0.24%
==========================================
Files 19 19
Lines 1525 1523 -2
==========================================
+ Hits 1288 1290 +2
+ Misses 237 233 -4 ☔ View full report in Codecov by Sentry. |
I'll check it out later today. I assume I can create the docs locally with |
yes |
It continued by throwing an error:
Edit: 1.11.2 also yields an error:
I have no idea what I have to do to fix those errors for either Julia version, so instead of rendering the markdown files, I am going through the markdown files individually in the branch. |
yeah, its a warning, not an error, it will work without examples just fine, I also don't have them since they take a lot of time to compute
Well this is interesting, I don't have this error, perhaps there is a bug or smth
Well this is jammer, I don't have these errors and CI seems to work just fine too, not sure what can be the reason... :( |
@abpolym Indeed https://discourse.julialang.org/t/issue-occurring-randomly-with-1-11/121233/4 |
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.
Still need to be able to see the proper rendering, but the changes look very nice/informative so far.
3. The interface through which the message is being computed | ||
4. The inference method being used (Belief Propagation or Variational Message Passing) | ||
|
||
The last point is particularly important - some message update rules may exist for Variational Message Passing (VMP) but not for Belief Propagation (BP), or vice versa. This is because VMP and BP use different mathematical formulations for computing messages. |
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.
For the last sentence This is because VMP and BP use different mathematical formulations for computing messages
, can we give the reader an explanation what "different mathematical formulations for computing messages" mean, s.t. (s)he can then hopefully immediately recognize the cause of the error?
Maybe we link to another part of our documentation that explains in more detail the difference BP vs VMP or equivalently exact bayesian inference vs variational inference?
We mention conjugacy (conjugate priors / conjugate pairs of distributions) in a few lines below without explanation, then again at the solution section with a link to wiki. Maybe we can just move all the detailed explanations into a separate document and tell the reader to read up on conjugate priors before creating models or when encountering this error?
- Avoids potential numerical instabilities from approximations | ||
- Throws an error when analytical solutions don't exist | ||
|
||
This means you may encounter `RuleNotFoundError` even in cases where approximate solutions could theoretically work. This is intentional - RxInfer will tell you explicitly when you need to consider alternative approaches (like those described in the Solutions section below) rather than silently falling back to potentially slower or less reliable approximations. |
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.
This means you may encounter
RuleNotFoundError even in cases where approximate solutions could theoretically work.
A reader might ask if it is then possible to let RxInfer explicitly try approximate solutions and how to do so, i.e. how to trigger variational inference? To immediately quench that thirst for someone that is impatient, can we mention right after this sentence that a reader can trigger approximations by following "these instructions", where "these instructions" just links forward to the "use approximations" subsection?
|
||
To better understand where message passing rules are needed, let's look at a simple factor graph visualization: | ||
|
||
```mermaid |
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.
This is cool! I don't know how it will look like after (HTML) rendering as I am currently looking at the markdown version - which in my case is a bit broken (I use dark theme so most of it is not visible, the μx→f text does not render properly and the text itself is cut off).
I will have to wait for the cairo-related error to be fixed/resolved, until I can view this mermaid graph in its full glory.
|
||
### 1. Convert to conjugate pairs | ||
|
||
First, try to reformulate your model using conjugate prior-likelihood pairs. Conjugate pairs have analytical solutions for message passing and are well-supported in RxInfer. For example, instead of using a Normal likelihood with Beta prior on its precision, use a Normal-Gamma conjugate pair. See [Conjugate prior - Wikipedia](https://en.wikipedia.org/wiki/Conjugate_prior#Table_of_conjugate_distributions) for a comprehensive list of conjugate distributions. |
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've been using code blocks for distributions like Normal
, Beta
everywhere but here 😄 .
|
||
You can enable stack depth limiting by passing it through the `options` parameter to the `infer` function: | ||
|
||
```@example stack-overflow-inference |
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.
Is it possible to use julia code blocks?
I think if I just add "julia" right after the three dashes "`" but before the (at)example then it should use color highlighting?
Test:
using RxInfer
@model function long_state_space_model(y)
x[1] ~ Normal(mean = 0.0, var = 1.0)
y[1] ~ Normal(mean = x[1], var = 1.0)
for i in 2:length(y)
x[i] ~ Normal(mean = x[i - 1], var = 1.0)
y[i] ~ Normal(mean = x[i], var = 1.0)
end
end
data = (y = rand(10000), )
using Test #hide
@test_throws StackOverflowError infer(model = long_state_space_model(), data = data) #hide
results = infer(
model = long_state_space_model(),
data = data,
options = (
limit_stack_depth = 100, # note the comma
)
)
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.
it uses highlighted julia code blocks with @example stack-overflow-inference
, see here, these are julia code blocks that are also being executed during the creation of the documentation page, just julia
will only highlight but not execute
|
||
For more details about inference options and execution, see: | ||
- [Static Inference](@ref manual-static-inference) documentation | ||
- The `options` parameter in the [`infer`](@ref) function documentation |
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 assume these references will render properly in HTML, in markdown it is broken 😢
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.
yes because there are not from markdown, but from Documenter.jl
Co-authored-by: Fons van der Plas <[email protected]>
Thanks @abpolym! |
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.
Given the incorporation of suggested changes from others, LGTM
Closes #60 #385
This was on our issue tracker for a while, but with the help of Cursor things get easier. Please review and suggest changes.
This pull request introduces several updates to the RxInfer documentation, focusing on improving user guidance and addressing common issues. The changes include adding a new "Sharp bits of RxInfer" section, updating the documentation to include references to this new section, and enhancing error handling and messaging within the inference process.
Documentation improvements:
docs/make.jl
: Introduced a draft mode for documentation generation, and added a new section "Sharp bits of RxInfer" to the documentation index. [1] [2]docs/src/manuals/comparison.md
: Added a note about using:=
for deterministic relationships in RxInfer.docs/src/manuals/model-specification.md
: Included a reference to the new "Sharp bits" section for further explanation on deterministic nodes.New "Sharp bits of RxInfer" section:
docs/src/manuals/sharpbits/overview.md
: Created an overview page for common pitfalls and issues in RxInfer, along with guidelines for contributing to this section.docs/src/manuals/sharpbits/rule-not-found.md
: Added detailed documentation on handlingRuleNotFoundError
, including causes, common scenarios, and solutions.docs/src/manuals/sharpbits/stack-overflow-inference.md
: Provided guidance on preventingStackOverflowError
during inference, with examples and performance considerations.docs/src/manuals/sharpbits/usage-colon-equality.md
: Explained the use of:=
for deterministic nodes and the reasoning behind not using=
.Enhanced error handling:
src/inference/inference.jl
: Improved error messaging forStackOverflowError
during inference, including suggestions for resolving the issue and links to relevant documentation. [1] [2]