docs: improve multiset_delta() docs
#830
Open
Conversation
This file contains 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
| /// released out. Conversely, `unique` take repeated items in, and only releases the new ones out. | ||
| /// | ||
| /// This operator does a similar inversion but for multiset semantics, with some caveats. When it | ||
| /// receives duplicate items, instead of ignoring them, it "subtracts" them from the items received |
There was a problem hiding this comment.
"subtracts" is confusing. Why not say "if we received k copies of an item in the previous tick, and we receive l > k copies in the current tick, we output l - k copies of the item."
| /// This operator does a similar inversion but for multiset semantics, with some caveats. When it | ||
| /// receives duplicate items, instead of ignoring them, it "subtracts" them from the items received | ||
| /// in the previous tick, and only releases new items if there are more than in the previous tick. | ||
| /// However unlike `unique`, this count is only maintained for the previous tick, not over all time. |
There was a problem hiding this comment.
Given the proposed rewording above, maybe rephrase to match.
I'm actually confused as to whether it remembers the MAX number of copies over time, or just the last tick number? If it's MAX, then we should probably implement a generic lattice_delta, where this is a specific one for multiset lattices. If it's not MAX, then I'd like to discuss semantics --- it'd be weird for this to be the only 1-tick windowed operator (other than next_tick).
There was a problem hiding this comment.
It is only the previous tick. The semantics become weird if it's not somewhere after a persist operator
| /// // 5, 3 | ||
| /// // First two "3"s are removed due to previous tick. | ||
| /// // 'c', 'a' | ||
| /// // First two 'a's are removed due to previous tick. |
There was a problem hiding this comment.
Give an example of the 3rd tick to clarify what confuses me above about MAX over all time vs last tick?
|
OK as a separate issue we should review semantics, understand how Shadaj is
using it, see if we can design something less weird
…On Mon, Jul 10, 2023 at 10:51 PM Mingwei Samuel ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In hydroflow_lang/src/graph/ops/multiset_delta.rs
<#830 (comment)>
:
> @@ -5,30 +5,43 @@ use super::{
WriteContextArgs, RANGE_0, RANGE_1,
};
-// TODO(mingwei): more doc
-/// Multiset delta from the previous tick.
+/// The multiset inverse of [`persist()`](#persist).
+///
+/// > 1 input stream of `T`, 1 output stream of `T`, where `T: Eq + Hash`
+///
+/// For set semantics, [`unique()`](#unique) can be thought of as a "delta" operator, the inverse
+/// of [`persist()`](#persist). In `persist`, new items come in, and all items are repeatedly
+/// released out. Conversely, `unique` take repeated items in, and only releases the new ones out.
+///
+/// This operator does a similar inversion but for multiset semantics, with some caveats. When it
+/// receives duplicate items, instead of ignoring them, it "subtracts" them from the items received
+/// in the previous tick, and only releases new items if there are more than in the previous tick.
+/// However unlike `unique`, this count is only maintained for the previous tick, not over all time.
It is only the previous tick. The semantics become weird if it's not
somewhere after a persist operator
—
Reply to this email directly, view it on GitHub
<#830 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAC54QJZNSPYFPL5OGZV4XTXPTSV7ANCNFSM6AAAAAA2E2MYJY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
MingweiSamuel
force-pushed
the
md-docs
branch
from
0cec25f to
845c2a5
Compare
MingweiSamuel
force-pushed
the
main
branch
2 times, most recently
from
e9a87eb to
6039078
Compare
MingweiSamuel
force-pushed
the
main
branch
4 times, most recently
from
4f84634 to
0e64791
Compare
MingweiSamuel
force-pushed
the
main
branch
2 times, most recently
from
fb8b3b3 to
a2ec110
Compare
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.
No description provided.