Skip to content

Interval::overlap_duration overflows on unbounded intervals; in release this zeroes the stochastic sampler's acceptance probability #4

Description

@cplonski20

Version: unirust-rs 0.2.0, commit 47b317cee11c793545416679087f015eaf33d677.

Interval::overlap_duration does not guard the NEG_INF/POS_INF sentinels, so two Interval::all_time() values overflow the subtraction at src/temporal.rs:116. Interval::duration() right above it does guard them and returns None.

Repro

use unirust_rs::temporal::Interval;

fn main() {
    let a = Interval::all_time();
    let b = Interval::all_time();
    println!("{}", a.overlap_duration(&b));
}

Expected: 0, or a saturating value, or None. Anything that is not negative.

Actual, debug:

thread 'main' panicked at src/temporal.rs:116:13:
attempt to subtract with overflow

Actual, release: -1.

Why the release value matters

src/linker.rs:786 uses this as a sampling weight:

let overlap = interval.overlap_duration(cand_interval);
let cand_len = cand_interval.duration_or_zero();
let min_len = interval_len.min(cand_len).max(1);
let weight = (overlap as f64 / min_len as f64).min(1.0);
...
hash_frac < base_prob * (0.5 + 0.5 * weight)

With two all_time() intervals, duration_or_zero() is 0 on both sides, so min_len is 1 and weight is -1.0. The acceptance threshold becomes base_prob * 0.0, which is exactly 0.0, and hash_frac < 0.0 is never true. Every candidate pair with two unbounded intervals is dropped rather than sampled, silently and with no metric to notice it by.

stochastic_sampling is true in every preset in src/config/tuning.rs, and the path engages once a block exceeds sampling_threshold (200 to 800 by preset).

Interval::all_time() is not an exotic input. It is the public constructor for an attribute with no known validity window, and the crate uses it itself at src/graph.rs:452 and :600. My data has person names with no validity window at all, so every name descriptor gets all_time(), and common-surname blocks are exactly the ones large enough to trigger sampling.

A min(cand_len) on the weight would not fix it on its own, since duration_or_zero() already returns 0 for these and min_len is floored at 1. The subtraction itself needs the guard.

Smaller, separate

allen_relation at src/temporal.rs:196 returns Finishes for the (Less, Equal) arm and FinishedBy for (Greater, Equal). Allen's definitions, and the crate's own doc comment on AllenRelation::Finishes (a.start > b.start && a.end == b.end), give the opposite:

assert_eq!(
    allen_relation(&Interval::new(0, 100)?, &Interval::new(50, 100)?),
    AllenRelation::FinishedBy
); // returns Finishes

Every other pair in that match is oriented correctly. test_allen_relations covers only Overlaps, Meets and MetBy. Happy to split this into its own issue if you prefer.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions