Skip to content

Make meet and join value bias independent of node layout - #181

Closed
imlvts wants to merge 3 commits into
Adam-Vandervorst:masterfrom
imlvts:fix/join-meet-value-bias-by-layout
Closed

imlvts wants to merge 3 commits into
Adam-Vandervorst:masterfrom
imlvts:fix/join-meet-value-bias-by-layout

Conversation

@imlvts

@imlvts imlvts commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

On a collision, join and meet keep the left operand's value (pjoin / pmeet on u64 return self). Which value survived actually depended on how each operand happened to be stored: when one side was a list node and
the other a byte node, the list node's value won whichever side it was on.

  let line  = { [1]: 0 }                 // list node
  let dense = { [0]: 0, [1]: 1, [2]: 0 } // byte node
  line.join(&dense)[1]   // was 1, now 0 (left operand)
  dense.join(&line)[1]   // 1

The same held for meet, meet_2, meet_into and join_into wherever a byte node meets a list node.

Changes

Make meet and join value bias independent of node layout

  • Join, list node on the left of a byte node: the list node used to be merged into a copy of the byte node, which made it the right operand. It is now copied into a byte node and joined byte-to-byte, so it stays on the
    left. This removes the old merge-into-a-copy arms and CellByteNode::clone_as_dense, which nothing else used.
  • Meet, byte node on the left of a list node: the list node runs the meet, because it is the side that can list its payloads. A swapped flag through pmeet_generic now makes it resolve each collision as other op self. Converting to a byte node doesn't work here: a byte-to-byte meet drops dangling paths, and map_meet_dangling_branching_factor_test1 requires them in both operand orders.
  • merge_guts: when one list node's payload is joined with the other's child, the payload now goes on the left.

Test that a join keeps a value across clone-shared structure
With b = a.clone() plus one value, a.join(&b) dropped that value and b.join(&a) kept it. The fix above resolves this; the test fails on master.

  a = { dangling 00000, 101, 103 }
  b = a.clone() + { 1000: 1 }
  a.join(&b)[1000]   // was None, now Some(1)

Drop an emptied child beside a value in a list-node meet
Once the meet runs with the operands the right way round, a list node holding a value and a child under the same key can meet the child down to an empty node. The result kept that empty child next to the value, which is an
invalid node and trips a debug assertion. The empty child is now dropped, because the value already keeps the path.

Testing

  • New tests:
    • meet_value_bias_is_left_regardless_of_node_layout: meet_2 and meet_into across list, byte and tiny layouts.
    • join_value_bias_is_left_regardless_of_node_layout: join and join_into, at the root and one level down.
    • map_join_keeps_value_across_shared_structure
    • map_meet_value_beside_emptied_child: catches the invalid node in debug builds.
  • The full suite passes in release and debug, and each commit passes on its own.
  • The algebraic equivalence fuzzer (2M cases) no longer reports any u64 value-bias findings. They came from routes that disagreed with PathMap::join / PathMap::meet: model, dnf, nary, ternary,
    subtract_zipper, and the zipper_join / OverlayZipper join strategies. The join value-loss cases also drop by more than half.

Not included

  • join_k_path_into still folds k-paths in its current order, so on a collision the value that survives may not come from the first path. That is a separate question about what the spec should say.
  • A few dangling-path results change for list-on-the-left joins, because a byte-to-byte join treats dangling children slightly differently from the old merge. In 2M fuzz cases this showed up only as a dozen dangling-path
    differences on the join_into_take strategy, never in values.

Meets and joins keep the left operand's value on a collision.  A list
node joining a byte node merged itself into a copy of the byte node, and
a byte node meeting a list node let the list node run the meet, so either
way the list node's values won wherever it was.  The join now goes
through a byte-node copy of the list node, and the meet tells the list
node it is the right operand.  merge_guts also joined a list node's
payload with the other slot's child on the right.
Finding 4 of the algebraic fuzzer: with b a clone of a plus one value,
a.join(&b) dropped that value while b.join(&a) kept it.  Fixed by the
value-bias change to which operand a byte-node/list-node join keeps.
When a list node holds a value and a child under the same key and the
child meets an empty node, the meet left an empty child (a dangling
sentinel) next to the value, which validate_node rejects.  The value
already keeps the path, so drop the sentinel.

Reachable once the value-bias fix put the empty node on the left of the
child meet: EmptyNode's meet keeps a dangling tip, while the other order
returned None.
@Adam-Vandervorst

Copy link
Copy Markdown
Owner

Algebraic ops should assume lattice values. Keeping the left operand is not, hence the behavior is undefined/op is unsupported.

@Adam-Vandervorst

Copy link
Copy Markdown
Owner

Please add the join fix separately to #178

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.

2 participants