Document cycle_fallback in the book #1056
Conversation
✅ Deploy Preview for salsa-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
book/src/cycles.md
Outdated
|
|
||
| ## Fallback Values | ||
|
|
||
| You can use `cycle_result` to specify a fallback value if Salsa detects a cycle. Salsa will ignore all the other queries and use this value directly. |
There was a problem hiding this comment.
To me it's not entirely clear what ignore all other queries means in this context. Are you trying to say that salsa discards the in-progress query result and instead returns fallback.
I think it might also be worth mentioning here that this can result in non-determinism because Salsa never enforces that all queries converge. But I don't fully remember the details here. But I think there was some discussion around it on the PR that introduced the feature
There was a problem hiding this comment.
@MichaReiser "ignore all other queries" is how it's described in #801 FWIW, and per #801 (comment) I think the final version of the PR is deterministic.
Are you trying to say that salsa discards the in-progress query result and instead returns fallback.
I think so? That's my understanding from a skim of the PR, but happy to tweak wording if you prefer something else :)
There was a problem hiding this comment.
I think the confusing part is the ignore all other queries. This makes it sound as if it throws away the result of any query called as part of the cyclic function. However, this is not the case. The only result that it throws away is the result of the cycle head (the query with the cycle_fallback), and instead, replaces the value with the fallback value.
Maybe an easier way to describe this is that a query with an cycle_result attribute always runs to completion, but its resulting value will be replaced with fallback value if it hits a cycle (instead of whatever value it ended up computing).
The non-determinism is described in this comment #798 (comment) (I think it's fine linking to that comment but it's worth pointing out that this is a limitation of this approach over proper fixpoint)
There was a problem hiding this comment.
@MichaReiser just to confirm, that's a different PR -- #798 vs #801 -- does #801 also have non-determinism?
There was a problem hiding this comment.
Yes, they both have the same non-determinism
This was added in salsa-rs#801 but wasn't previously documented.
Wilfred
force-pushed
the
document_cycle_fallback
branch
from
ff28ff3 to
2fdfb3e
Compare
|
OK, expanded and clarified following the comments here. |
| } | ||
| ``` | ||
|
|
||
| Unlike fixpoint iteration, queries attributed with `cycle_fallback` also use their fallback value if |
There was a problem hiding this comment.
I learned more about cycle_fallback and am now convinced that it doesn't have any non-deterministic behavior. I updated the section here
2 participants
This was added in #801 but wasn't previously documented.