Skip to content

Meet: prune leaves no dangling path below the focus; status follows values - #180

Open
imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:fix/meet-prune-no-dangling
Open

imlvts wants to merge 1 commit into
Adam-Vandervorst:masterfrom
imlvts:fix/meet-prune-no-dangling

Conversation

@imlvts

@imlvts imlvts commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

meet_into and meet_k_path_into with prune = true now remove every dangling path below the focus, not only the chain at the focus, then prune the focus as before. With prune = false a meet may leave dangling paths (which ones is unspecified), but the status and return value depend only on the values: a result holding none is None / false. meet_2, which has no prune flag, follows prune = false.

Two tests encoded the old rule and are updated: write_zipper_meet_into_test4 kept a dangling-only intersection under prune = true and reported Element, and write_zipper_prune_flag_test assumed prune = true equals prune = false plus prune_path at the focus, which no longer holds for meet_into.

…alues

meet_into and meet_k_path_into with prune = true now remove every dangling
path below the focus, not only the chain at the focus, then prune the
focus as before.  With prune = false a meet may leave dangling paths
(which ones is unspecified), but the status and return value depend only
on the values: a result holding none is None / false.  meet_2, which has
no prune flag, follows prune = false.

Two tests encoded the old rule and are updated: write_zipper_meet_into_test4
kept a dangling-only intersection under prune = true and reported Element,
and write_zipper_prune_flag_test assumed prune = true equals prune = false
plus prune_path at the focus, which no longer holds for meet_into.
@luketpeterson

luketpeterson commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

This change in behavior is (at least partially) counter-productive, IMO.

The rationale behind meet not giving first-class treatment to dangling paths is performance. But instituting a reliable policy of "no dangling path survives a meet" (which this PR does) is just as costly to maintain as first-class treatment of dangling paths in meet. (Actually more costly within the meet call, it's just that it hopefully pays back in future traversals avoided.)

Additionally, this PR causes meet_into's prune flag to behave differently from the flag on every other op. Every other op's prune flag is documented to be identical to calling z.prune_path() after the op, which does not go below the focus looking for dangling paths / subtries.

@luketpeterson

luketpeterson commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

This PR is as good a place to have the discussion as any, since the other meet behavior PR is closed.

First, let me introduce a theoretical API concept: trim or shake or strip. Name to-be-decided, but I'll call it trim for now. prune is upward-looking, focussed on a single path. trim is downward-looking and affects the whole subtrie.

If trimming a subtrie is desirable, merging a trim walk with a meet walk means we can accomplish both goals with less work than the sum of the two operations separately. I'll call that meet_trim.

cost(meet_trim) < cost(meet) + cost(trim)  But
cost(meet_trim) > cost(meet)

But a thorough meet_trim cannot take advantage of the meet(t, t) == t optimization. It still needs to walk the subtrie with trim behavior. And do some gymnastics to make sure it doesn't accidentally end up copy-uniquing more of the trie than is necessary in the process.

Which gets to a third thing that I'll call meet_trim_dirty for now (we probably want a better name). meet_trim_dirty costs roughly the same as a meet that respects dangling paths, and trims when it's already writing into that node, but doesn't visit branches that it doesn't need to, by applying the same early-outs as the "clean" meet. If exhaustive trimming isn't required for correctness, but trimming helps contain costs down the line, meet_trim_dirty might be the best policy.

cost(meet_trim_dirty) ≃ cost(meet)

So there are 3 possible contracts for a meet-like operation.

  • meet_trim
  • meet (clean version that treats all paths as first-class)
  • meet_trim_dirty makes no guarantees about trimming, but trims when it's cheap

Each one has a situation where it's the clear winner. I hate that the contract for meet_trim_dirty is hard to specify, but that could be a c'est la vie situation.

I guess if I had my preference, it would be to implement meet the "clean" way with full guarantees around first class paths, and provide a meet_trim_dirty API because that's what MORK seems to want for maximum performance. And save the meet_trim API for somebody to ask for it, because it sounds tricky to get right from a "don't break sharing" standpoint.

@Adam-Vandervorst

Copy link
Copy Markdown
Owner

it would be to implement meet the "clean" way with full guarantees around first class paths

What does it mean to have first class paths in meet? What guaranteed do you want to provide?

provide a meet_trim_dirty API because that's what MORK seems to want for maximum performance

I'd have expected this phrasing for e.g. zipper re-use, not for a huge asymptotic gap in the algebra.


If we want to first-class dangling paths, we can, but then a lot is going to change with a lot of work; so there'd have to be a use case (e.g. someone asking about dangling paths under algebraic ops)

@luketpeterson

luketpeterson commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

What does it mean to have first class paths in meet? What guaranteed do you want to provide?

It means meet upholds these laws:

nothing ∩ nothing = nothing
nothing ∩ dangling_path = nothing
nothing ∩ path_with_value = nothing
dangling_path ∩ dangling_path = dangling_path
dangling_path ∩ path_with_value = dangling_path
path_with_value ∩ path_with_value = path_with_value

I'd have expected this phrasing (dirty) for e.g. zipper re-use, not for a huge asymptotic gap in the algebra.

I call it "dirty" because it makes vastly fewer guarantees about the behavior or the final result. The guarantees of meet_trim_dirty would be:

nothing ∩ nothing = nothing
nothing ∩ path_with_value = nothing
path_with_value ∩ path_with_value = path_with_value

Therefore there are 3 other cases are:

nothing ∩ dangling_path = nothing | dangling_path (unreliable which)
dangling_path ∩ dangling_path = nothing | dangling_path (unreliable which)
dangling_path ∩ path_with_value = nothing | dangling_path (unreliable which)

We can choose a different word from dirty. But the point is, it makes fewer guarantees about the output that the caller can rely on.

For completeness, the guarantees of meet_trim are:

nothing ∩ nothing = nothing
nothing ∩ dangling_path = nothing
nothing ∩ path_with_value = nothing
dangling_path ∩ dangling_path = nothing
dangling_path ∩ path_with_value = nothing
path_with_value ∩ path_with_value = path_with_value

If we want to first-class dangling paths, we can, but then a lot is going to change with a lot of work; so there'd have to be a use case (e.g. someone asking about dangling paths under algebraic ops)

What would need to change? It's practically what we already have minus what essentially can be thought of as "bugs". I.e. there is certainly nobody currently relying on dangling paths not being first class, because the current implementation is somewhere between the "first_class_dangling_paths" implementation and the meet_trim_dirty

@luketpeterson

Copy link
Copy Markdown
Collaborator

Also worth saying - we could extend the other ops (join, subtract, etc.) to also include _trim and _trim_dirty flavors.

If we're working from the pragmatic frame that "having dangling paths is a cost, so they should be cleaned up opportunistically, but spending immediate resources to exhaustively trim them out is not a good tradeoff", having _trim_dirty variants of the other ops makes sense.

@imlvts

imlvts commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

I don't like the idea of exploding the API surface with extra names.
I would suggest something like prune={Never,Opportunistic,Strict}.
Never = leave dangling paths
Opportunistic = remove dangling paths when convenient & doesn't lose perf (no promises about dangling paths are made here)
Strict = slower, logically same as running op(...prune=Never); prune();

@luketpeterson

luketpeterson commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

I don't like the idea of exploding the API surface with extra names. I would suggest something like prune={Never,Opportunistic,Strict}. Never = leave dangling paths Opportunistic = remove dangling paths when convenient & doesn't lose perf Strict = slower, logically same as running op(...prune=Never); prune();

prune != trim. They're conceptually related (hence the use of near synonyms in English), but it's important not to mix them up because (if we adopt trim as canonical language) they will do very different things.

Regarding arg vs. additional entry point, I have a couple of opinions / observations. But it's mainly an aesthetic taste thing.

  1. More flavors of an operation vs adding an arg that changes behavior is the same API surface area. It's just a question of how the surface area is presented to the user. I think I have an aesthetic preference for the "more flavors" version over the arg. But it's not a hill I would die on.

  2. The behavior selector is fundamentally a policy. We we know we're going to redo the alg-ops API. And when we are able to specify a subtrie-subtrie policy, the "trim_opportunistic" behavior will be implementable. So any choice we make regarding how the API expresses the concept is temporary.

trim_opportunistic sounds less pejorative than trim_dirty, so we can start calling it that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants