Skip to content

Commit 87b04a0

Browse files
committed
concrete: directly use PositiveF64 in Policy::enumerate_pol
By changing this one function and chasing compiler errors, I found I was able to (and required to) replace a *ton* of f64s with PositiveF64s. Amazingly, this did not require adding any new functionality to the PositiveF64 type, or any conversions. Instead, I was able to *delete* a ton of conversions, wrappers, unwrapping, etc. Now concrete.rs has literally no instances of 'f64' which is awesome. The next step is compiler.rs.
1 parent 3c8942a commit 87b04a0

2 files changed

Lines changed: 54 additions & 41 deletions

File tree

src/policy/concrete.rs

Lines changed: 40 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,11 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
348348
let tap_tree = match policy {
349349
Self::Trivial => None,
350350
policy => {
351-
let leaves =
352-
policy.enumerate_leaves(1.0, max_leaves, Self::enumerate_pol_native);
351+
let leaves = policy.enumerate_leaves(
352+
PositiveF64::ONE,
353+
max_leaves,
354+
Self::enumerate_pol_native,
355+
);
353356
let n = leaves.len();
354357
if n > max_leaves {
355358
return Err(CompilerError::TooManyTapleaves { n, max: max_leaves });
@@ -365,7 +368,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
365368
leaf_index: leaf_idx,
366369
});
367370
}
368-
leaf_compilations.push((PositiveF64(*prob), compilation));
371+
leaf_compilations.push((*prob, compilation));
369372
}
370373
if !leaf_compilations.is_empty() {
371374
Some(with_huffman_tree::<Pk>(leaf_compilations))
@@ -417,14 +420,11 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
417420
Self::Trivial => None,
418421
policy => {
419422
let leaf_compilations: Vec<_> = policy
420-
.enumerate_policy_tree(1.0)
423+
.enumerate_policy_tree(PositiveF64::ONE)
421424
.into_iter()
422425
.filter(|x| x.1 != Arc::new(Self::Unsatisfiable))
423426
.map(|(prob, pol)| {
424-
(
425-
PositiveF64(prob),
426-
compiler::best_compilation(pol.as_ref()).unwrap(),
427-
)
427+
(prob, compiler::best_compilation(pol.as_ref()).unwrap())
428428
})
429429
.collect();
430430

@@ -502,20 +502,20 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
502502
/// disjunction over sub-policies output by it. The probability calculations are similar
503503
/// to [`Policy::tapleaf_probability_iter`].
504504
#[cfg(feature = "compiler")]
505-
fn enumerate_pol(&self, prob: f64) -> Vec<(f64, Arc<Self>)> {
505+
fn enumerate_pol(&self, prob: PositiveF64) -> Vec<(PositiveF64, Arc<Self>)> {
506506
match self {
507507
Self::Or(subs) => {
508508
let normalized_iter = PositiveF64::normalized_iter(subs.iter().map(|x| x.0.into()));
509509
normalized_iter
510510
.zip(subs.iter())
511-
.map(|(odds, (_, pol))| (prob * f64::from(odds), pol.clone()))
511+
.map(|(odds, (_, pol))| (prob * odds, pol.clone()))
512512
.collect::<Vec<_>>()
513513
}
514514
Self::Thresh(ref thresh) if thresh.is_or() => {
515-
let total_odds = thresh.n();
515+
let total_odds = PositiveF64::n(thresh);
516516
thresh
517517
.iter()
518-
.map(|pol| (prob / total_odds as f64, pol.clone()))
518+
.map(|pol| (prob / total_odds, pol.clone()))
519519
.collect::<Vec<_>>()
520520
}
521521
Self::Thresh(ref thresh) if !thresh.is_and() => generate_combination(thresh, prob),
@@ -528,26 +528,26 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
528528
/// This ensures nested `Or` branches inside `And` nodes are decomposed into
529529
/// separate sub-policies, producing IF-free leaves for Taptree-native compilation.
530530
#[cfg(feature = "compiler")]
531-
fn enumerate_pol_native(&self, prob: f64) -> Vec<(f64, Arc<Self>)> {
531+
fn enumerate_pol_native(&self, prob: PositiveF64) -> Vec<(PositiveF64, Arc<Self>)> {
532532
match self {
533533
Self::Or(subs) => {
534534
let normalized_iter = PositiveF64::normalized_iter(subs.iter().map(|x| x.0.into()));
535535
normalized_iter
536536
.zip(subs.iter())
537-
.map(|(odds, (_, pol))| (prob * f64::from(odds), pol.clone()))
537+
.map(|(odds, (_, pol))| (prob * odds, pol.clone()))
538538
.collect::<Vec<_>>()
539539
}
540540
Self::Thresh(ref thresh) if thresh.is_or() => {
541-
let total_odds = thresh.n();
541+
let total_odds = PositiveF64::n(thresh);
542542
thresh
543543
.iter()
544-
.map(|pol| (prob / total_odds as f64, pol.clone()))
544+
.map(|pol| (prob / total_odds, pol.clone()))
545545
.collect::<Vec<_>>()
546546
}
547547
Self::Thresh(ref thresh) if !thresh.is_and() => generate_combination(thresh, prob),
548548
Self::And(subs) => {
549549
for (i, sub) in subs.iter().enumerate() {
550-
let child_expanded = sub.enumerate_pol_native(1.0);
550+
let child_expanded = sub.enumerate_pol_native(PositiveF64::ONE);
551551
if child_expanded.len() > 1 {
552552
let other: Vec<_> = subs
553553
.iter()
@@ -578,7 +578,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
578578
/// set](`BTreeSet`) of `(prob, policy)` (ordered by probability) to maintain the list of
579579
/// enumerated sub-policies whose disjunction is isomorphic to initial policy (*invariant*).
580580
#[cfg(feature = "compiler")]
581-
fn enumerate_policy_tree(self, prob: f64) -> Vec<(f64, Arc<Self>)> {
581+
fn enumerate_policy_tree(self, prob: PositiveF64) -> Vec<(PositiveF64, Arc<Self>)> {
582582
self.enumerate_leaves(prob, MAX_COMPILATION_LEAVES, Self::enumerate_pol)
583583
}
584584

@@ -590,10 +590,10 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
590590
#[allow(clippy::type_complexity)]
591591
fn enumerate_leaves(
592592
self,
593-
prob: f64,
593+
prob: PositiveF64,
594594
max_leaves: usize,
595-
expand_fn: fn(&Self, f64) -> Vec<(f64, Arc<Self>)>,
596-
) -> Vec<(f64, Arc<Self>)> {
595+
expand_fn: fn(&Self, PositiveF64) -> Vec<(PositiveF64, Arc<Self>)>,
596+
) -> Vec<(PositiveF64, Arc<Self>)> {
597597
let mut tapleaf_prob_vec = BTreeSet::<(Reverse<PositiveF64>, Arc<Self>)>::new();
598598
// Store probability corresponding to policy in the enumerated tree. This is required since
599599
// owing to the current [policy element enumeration algorithm][`Policy::enumerate_pol`],
@@ -602,8 +602,8 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
602602
let mut pol_prob_map = BTreeMap::<Arc<Self>, PositiveF64>::new();
603603

604604
let arc_self = Arc::new(self);
605-
tapleaf_prob_vec.insert((Reverse(PositiveF64(prob)), Arc::clone(&arc_self)));
606-
pol_prob_map.insert(Arc::clone(&arc_self), PositiveF64(prob));
605+
tapleaf_prob_vec.insert((Reverse(prob), Arc::clone(&arc_self)));
606+
pol_prob_map.insert(Arc::clone(&arc_self), prob);
607607

608608
// Since we know that policy enumeration *must* result in increase in total number of nodes,
609609
// we can maintain the length of the ordered set to check if the
@@ -613,21 +613,21 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
613613
// store the variables
614614
let mut enum_len = tapleaf_prob_vec.len();
615615

616-
let mut ret: Vec<(f64, Arc<Self>)> = vec![];
616+
let mut ret: Vec<(PositiveF64, Arc<Self>)> = vec![];
617617

618618
// Stopping condition: When NONE of the inputs can be further enumerated.
619619
'outer: loop {
620620
//--- FIND a plausible node ---
621621
let mut prob: Reverse<PositiveF64> = Reverse(PositiveF64(0.0));
622622
let mut curr_policy: Arc<Self> = Arc::new(Self::Unsatisfiable);
623-
let mut curr_pol_replace_vec: Vec<(f64, Arc<Self>)> = vec![];
623+
let mut curr_pol_replace_vec: Vec<(PositiveF64, Arc<Self>)> = vec![];
624624
let mut no_more_enum = false;
625625

626626
// The nodes which can't be enumerated further are directly appended to ret and removed
627627
// from the ordered set.
628-
let mut to_del: Vec<(f64, Arc<Self>)> = vec![];
628+
let mut to_del: Vec<(PositiveF64, Arc<Self>)> = vec![];
629629
'inner: for (i, (p, pol)) in tapleaf_prob_vec.iter().enumerate() {
630-
curr_pol_replace_vec = expand_fn(pol, p.0 .0);
630+
curr_pol_replace_vec = expand_fn(pol, p.0);
631631
enum_len += curr_pol_replace_vec.len() - 1; // A disjunctive node should have separated this into more nodes
632632
assert!(prev_len <= enum_len);
633633

@@ -644,14 +644,14 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
644644
// Either node is enumerable, or we have
645645
// Mark all non-enumerable nodes to remove,
646646
// if not returning value in the current iteration.
647-
to_del.push((p.0 .0, Arc::clone(pol)));
647+
to_del.push((p.0, Arc::clone(pol)));
648648
}
649649
}
650650

651651
// --- Sanity Checks ---
652652
if enum_len > max_leaves || no_more_enum {
653653
for (p, pol) in tapleaf_prob_vec.into_iter() {
654-
ret.push((p.0 .0, pol));
654+
ret.push((p.0, pol));
655655
}
656656
break 'outer;
657657
}
@@ -665,7 +665,7 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
665665

666666
// OPTIMIZATION - Move marked nodes into final vector
667667
for (p, pol) in to_del {
668-
assert!(tapleaf_prob_vec.remove(&(Reverse(PositiveF64(p)), pol.clone())));
668+
assert!(tapleaf_prob_vec.remove(&(Reverse(p), pol.clone())));
669669
pol_prob_map.remove(&pol);
670670
ret.push((p, pol.clone()));
671671
}
@@ -675,13 +675,12 @@ impl<Pk: MiniscriptKey> Policy<Pk> {
675675
match pol_prob_map.get(&policy) {
676676
Some(prev_prob) => {
677677
assert!(tapleaf_prob_vec.remove(&(Reverse(*prev_prob), policy.clone())));
678-
tapleaf_prob_vec
679-
.insert((Reverse(PositiveF64(prev_prob.0 + p)), policy.clone()));
680-
pol_prob_map.insert(policy.clone(), PositiveF64(prev_prob.0 + p));
678+
tapleaf_prob_vec.insert((Reverse(prev_prob + p), policy.clone()));
679+
pol_prob_map.insert(policy.clone(), prev_prob + p);
681680
}
682681
None => {
683-
tapleaf_prob_vec.insert((Reverse(PositiveF64(p)), policy.clone()));
684-
pol_prob_map.insert(policy.clone(), PositiveF64(p));
682+
tapleaf_prob_vec.insert((Reverse(p), policy.clone()));
683+
pol_prob_map.insert(policy.clone(), p);
685684
}
686685
}
687686
}
@@ -1181,12 +1180,12 @@ fn with_huffman_tree<Pk: MiniscriptKey>(
11811180
#[cfg(feature = "compiler")]
11821181
fn generate_combination<Pk: MiniscriptKey>(
11831182
thresh: &Threshold<Arc<Policy<Pk>>, 0>,
1184-
prob: f64,
1185-
) -> Vec<(f64, Arc<Policy<Pk>>)> {
1183+
prob: PositiveF64,
1184+
) -> Vec<(PositiveF64, Arc<Policy<Pk>>)> {
11861185
debug_assert!(thresh.k() < thresh.n());
11871186

1188-
let prob_over_n = prob / thresh.n() as f64;
1189-
let mut ret: Vec<(f64, Arc<Policy<Pk>>)> = vec![];
1187+
let prob_over_n = prob / PositiveF64::n(thresh);
1188+
let mut ret: Vec<(PositiveF64, Arc<Policy<Pk>>)> = vec![];
11901189
for i in 0..thresh.n() {
11911190
let thresh_less_1 = Threshold::from_iter(
11921191
thresh.k(),
@@ -1288,7 +1287,7 @@ mod compiler_tests {
12881287
.collect();
12891288
let thresh = Threshold::new(2, policies).unwrap();
12901289

1291-
let combinations = generate_combination(&thresh, 1.0);
1290+
let combinations = generate_combination(&thresh, PositiveF64::ONE);
12921291

12931292
let comb_a: Vec<Policy<String>> = vec![
12941293
policy_str!("pk(B)"),
@@ -1315,7 +1314,7 @@ mod compiler_tests {
13151314
.map(|sub_pol| {
13161315
let expected_thresh =
13171316
Threshold::from_iter(2, sub_pol.into_iter().map(Arc::new)).unwrap();
1318-
(0.25, Arc::new(Policy::Thresh(expected_thresh)))
1317+
(PositiveF64::ONE_QUARTER, Arc::new(Policy::Thresh(expected_thresh)))
13191318
})
13201319
.collect::<Vec<_>>();
13211320
assert_eq!(combinations, expected_comb);

src/primitives/positive_f64.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ impl PositiveF64 {
1515
/// The constant one.
1616
pub const ONE: Self = Self(1.0);
1717

18+
/// Constant used in unit tsets
19+
#[cfg(test)]
20+
pub const ONE_QUARTER: Self = Self(0.25);
21+
1822
/// Takes an iterator over [`PositiveF64`] and produces a new iterator where
1923
/// each item is divided so that they all total to 1.
2024
///
@@ -72,6 +76,16 @@ impl ops::Add for PositiveF64 {
7276
fn add(self, rhs: Self) -> Self::Output { Self(self.0 + rhs.0) }
7377
}
7478

79+
impl ops::Add<PositiveF64> for &PositiveF64 {
80+
type Output = PositiveF64;
81+
fn add(self, rhs: PositiveF64) -> Self::Output { PositiveF64(self.0 + rhs.0) }
82+
}
83+
84+
impl ops::Add<&Self> for PositiveF64 {
85+
type Output = Self;
86+
fn add(self, rhs: &Self) -> Self::Output { Self(self.0 + rhs.0) }
87+
}
88+
7589
impl ops::Mul for PositiveF64 {
7690
type Output = Self;
7791
fn mul(self, rhs: Self) -> Self::Output { Self(self.0 * rhs.0) }

0 commit comments

Comments
 (0)