fix: fix kernel bfs resolver issue

kernel/lcrconn/src/dataset.rs

@@ -186,8 +186,18 @@ impl Dataset {
         Ok(())
     }
 
+    /// The number of available standard values.
+    pub fn len(&self) -> usize {
+        self.items.len()
+    }
+
+    /// Get the available standard value by index.
+    pub fn get(&self, index: usize) -> Option<f64> {
+        self.items.get(index).map(|i| i.value)
+    }
+
     /// Get the available standard values as an iterator of `f64`.
-    pub fn values(&self) -> impl Iterator<Item = f64> {
+    pub fn values(&self) -> impl Iterator<Item = f64> + Clone {
         self.items.iter().map(|i| i.value)
     }
 }

kernel/lcrconn/src/lib.rs

@@ -2,13 +2,3 @@ pub mod common;
 pub mod dataset;
 pub mod query;
 pub mod resolver;
-
-pub use common::{
-    Circuit, CircuitDeviceScale, CircuitCalculator, DeviceKind, JointKind, LcrConnError, SubCircuit,
-};
-pub use dataset::{
-    from_human_readable_value, get_human_readable_value_scale, to_human_readable_value, Dataset,
-    DatasetCollection, DatasetItem, UnitScale,
-};
-pub use query::{Request, Response, ResponseItem, ResponsePriority, MAX_RESPONSE_CNT};
-pub use resolver::{BfsResolver, LutResolver, Resolver};

kernel/lcrconn/src/resolver.rs

@@ -0,0 +1,26 @@
+pub mod bfs;
+pub mod lut;
+
+use crate::query::{Request, Response};
+use thiserror::Error as TeError;
+
+/// Aggregated error occurs in every resolvers.
+#[derive(Debug, TeError)]
+pub enum ResolverError {
+    #[error("{0}")]
+    BfsResolver(#[from] bfs::BfsResolverError),
+    #[error("a")]
+    LutResolver,
+}
+
+/// Abstract base trait for all resolvers.
+pub trait Resolver {
+    /// Resolve the request and return the response.
+    ///
+    /// `request` is the request to resolve.
+    /// The response containing the best matching circuits.
+    fn resolve(&self, request: &Request) -> Result<Response, ResolverError>;
+}
+
+pub use bfs::BfsResolver;
+pub use lut::LutResolver;

kernel/lcrconn/src/resolver/bfs.rs

@@ -1,251 +1,30 @@
+use super::{Resolver, ResolverError};
+use crate::common::{
+    Circuit, CircuitCalculator, CircuitCalculatorError, CircuitError, DeviceKind, JointKind,
+};
+use crate::dataset::{Dataset, DatasetCollection};
+use crate::query::{Request, Response, ResponseError};
+use itertools::Itertools;
+use ordered_float::OrderedFloat;
 use std::cmp::Ordering;
 use std::collections::BinaryHeap;
-use std::iter::FusedIterator;
+use strum::IntoEnumIterator;
+use thiserror::Error as TeError;
 
-use super::Resolver;
+/// Error occurs BFS resolver.
-use crate::common::{Circuit, CircuitCalculator, DeviceKind, JointKind, LcrConnError};
+#[derive(Debug, TeError)]
-use crate::dataset::{Dataset, DatasetCollection, DatasetItem};
+pub enum BfsResolverError {
-use crate::query::{Request, Response};
+    #[error("failed to build circuit: {0}")]
-
+    Circuit(#[from] CircuitError),
-// ============================================================================
+    #[error("failed on computing circuit properties: {0}")]
-// Lazy iterator structs for circuit generation
+    CircuitCalculator(#[from] CircuitCalculatorError),
-// ============================================================================
+    #[error("the size of binary heap {0} is invalid")]
-
+    BadBinHeapSize(usize),
-// YYC MARK:
+    #[error("fail to build response: {0}")]
-// Some circuit are equivalent in topology.
+    Response(#[from] ResponseError),
-// If we deduplicate these equaivalent circuit in building result,
-// there are too complex works.
-// So we should deduplicated these equivalent circuit at the beginning,
-// i.e. when generating them.
-// So following iterator structs are taking this job.
-
-/// Iterator over all possible one-device circuits without repeating equivalent topology.
-pub struct OneDeviceCircuitIter<'a> {
-    items: &'a [DatasetItem],
-    pos: usize,
 }
 
-impl<'a> OneDeviceCircuitIter<'a> {
+// region: BFS Item
-    pub fn new(items: &'a [DatasetItem]) -> Self {
-        Self { items, pos: 0 }
-    }
-}
-
-impl Iterator for OneDeviceCircuitIter<'_> {
-    type Item = Circuit;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.pos < self.items.len() {
-            // Every single device is unique so we directly output them.
-            // This feature is insured by dataset itself.
-            let circuit = Circuit::from_one_device(self.items[self.pos].value);
-            self.pos += 1;
-            Some(circuit)
-        } else {
-            None
-        }
-    }
-}
-
-impl FusedIterator for OneDeviceCircuitIter<'_> {}
-
-/// Iterator over all possible two-device circuits without repeating equivalent topology.
-pub struct TwoDeviceCircuitIter<'a> {
-    items: &'a [DatasetItem],
-    i: usize,
-    j: usize,
-    joint_idx: usize,
-}
-
-impl<'a> TwoDeviceCircuitIter<'a> {
-    pub fn new(items: &'a [DatasetItem]) -> Self {
-        Self {
-            items,
-            i: 0,
-            j: 0,
-            joint_idx: 0,
-        }
-    }
-}
-
-impl Iterator for TwoDeviceCircuitIter<'_> {
-    type Item = Circuit;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let n = self.items.len();
-        if n == 0 {
-            return None;
-        }
-
-        loop {
-            if self.joint_idx < JointKind::ALL.len() {
-                let jk = JointKind::ALL[self.joint_idx];
-                self.joint_idx += 1;
-                // The two devices in this circuit is always swapable,
-                // so we iterate them without repeating.
-                return Some(Circuit::from_two_devices(
-                    self.items[self.i].value,
-                    self.items[self.j].value,
-                    jk,
-                ));
-            }
-
-            // Advance to next combination
-            self.joint_idx = 0;
-            self.j += 1;
-            if self.j >= n {
-                self.i += 1;
-                self.j = self.i;
-                if self.i >= n {
-                    return None;
-                }
-            }
-        }
-    }
-}
-
-impl FusedIterator for TwoDeviceCircuitIter<'_> {}
-
-/// Iterator over three-device circuits where both joints share the same type.
-///
-/// In this case, all 3 devices are swapable and are iterated without repeating.
-pub struct ThreeDeviceSameJointIter<'a> {
-    items: &'a [DatasetItem],
-    i: usize,
-    j: usize,
-    k: usize,
-    joint_idx: usize,
-}
-
-impl<'a> ThreeDeviceSameJointIter<'a> {
-    pub fn new(items: &'a [DatasetItem]) -> Self {
-        Self {
-            items,
-            i: 0,
-            j: 0,
-            k: 0,
-            joint_idx: 0,
-        }
-    }
-}
-
-impl Iterator for ThreeDeviceSameJointIter<'_> {
-    type Item = Circuit;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let n = self.items.len();
-        if n == 0 {
-            return None;
-        }
-
-        loop {
-            if self.joint_idx < JointKind::ALL.len() {
-                let jk = JointKind::ALL[self.joint_idx];
-                self.joint_idx += 1;
-                return Some(Circuit::from_three_devices(
-                    self.items[self.i].value,
-                    self.items[self.j].value,
-                    jk,
-                    self.items[self.k].value,
-                    jk,
-                ));
-            }
-
-            self.joint_idx = 0;
-            self.k += 1;
-            if self.k >= n {
-                self.j += 1;
-                self.k = self.j;
-                if self.j >= n {
-                    self.i += 1;
-                    self.j = self.i;
-                    self.k = self.i;
-                    if self.i >= n {
-                        return None;
-                    }
-                }
-            }
-        }
-    }
-}
-
-impl FusedIterator for ThreeDeviceSameJointIter<'_> {}
-
-/// Iterator over three-device circuits where the two joint types differ.
-///
-/// In this case, the first 2 devices are swapable and are iterated without repeating,
-/// while the third device iterates over all values independently.
-pub struct ThreeDeviceDiffJointIter<'a> {
-    items: &'a [DatasetItem],
-    i: usize,
-    j: usize,
-    k: usize,
-    joint_idx: usize,
-}
-
-impl<'a> ThreeDeviceDiffJointIter<'a> {
-    pub fn new(items: &'a [DatasetItem]) -> Self {
-        Self {
-            items,
-            i: 0,
-            j: 0,
-            k: 0,
-            joint_idx: 0,
-        }
-    }
-}
-
-impl Iterator for ThreeDeviceDiffJointIter<'_> {
-    type Item = Circuit;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let n = self.items.len();
-        if n == 0 {
-            return None;
-        }
-
-        loop {
-            if self.joint_idx < JointKind::ALL.len() {
-                let j = JointKind::ALL[self.joint_idx];
-                self.joint_idx += 1;
-                return Some(Circuit::from_three_devices(
-                    self.items[self.i].value,
-                    self.items[self.j].value,
-                    j,
-                    self.items[self.k].value,
-                    j.flip(),
-                ));
-            }
-
-            self.joint_idx = 0;
-            self.k += 1;
-            if self.k >= n {
-                self.j += 1;
-                self.k = 0;
-                if self.j >= n {
-                    self.i += 1;
-                    self.j = self.i;
-                    self.k = 0;
-                    if self.i >= n {
-                        return None;
-                    }
-                }
-            }
-        }
-    }
-}
-
-impl FusedIterator for ThreeDeviceDiffJointIter<'_> {}
-
-/// Type alias for the chained three-device circuit iterator.
-pub type ThreeDeviceCircuitIter<'a> = std::iter::Chain<
-    ThreeDeviceSameJointIter<'a>,
-    ThreeDeviceDiffJointIter<'a>,
->;
-
-// ============================================================================
-// BfsItem
-// ============================================================================
 
 /// The entry used in BFS iteration storing circuit and value.
 pub struct BfsItem {
@@ -259,13 +38,11 @@ pub struct BfsItem {
 
 impl BfsItem {
     /// Create a new BFS item by computing values eagerly.
-    ///
+    pub fn new(circuit: Circuit, ccalc: &CircuitCalculator) -> Result<Self, BfsResolverError> {
-    /// # Errors
+        // YYC MARK:
-    ///
+        // The same reason for replacing cached_property like I done in `ResponseItem`.
-    /// See [`CircuitValueTrait::value`].
-    pub fn new(circuit: Circuit, ccalc: &CircuitCalculator) -> Result<Self, LcrConnError> {
         let value = ccalc.value(&circuit)?;
-        let unsigned_difference = ccalc.unsigned_difference(&circuit, Some(value))?;
+        let unsigned_difference = ccalc.unsigned_difference(&circuit, Some(value), None)?;
         Ok(Self {
             circuit,
             value,
@@ -294,30 +71,42 @@ impl BfsItem {
     }
 }
 
-// ============================================================================
+// endregion
-// ResultBucket
+
-// ============================================================================
+// region Result Bucket
 
 /// An item stored in a [`ResultBucket`].
 struct ResultBucketItem {
     /// The score associated with this item.
-    score: f64,
+    score: OrderedFloat<f64>,
-    /// The underlying BfsItem.
+    /// The underlying [BfsItem].
     item: BfsItem,
     /// Monotonic counter used as a tiebreaker when scores are equal,
-    /// ensuring that BinaryHeap never compares BfsItem directly.
+    /// ensuring that BinaryHeap never compares [BfsItem] directly.
     seq: usize,
 }
 
 impl ResultBucketItem {
-    fn new(score: f64, item: BfsItem, seq: usize) -> Self {
+    pub fn new(score: f64, item: BfsItem, seq: usize) -> Self {
-        Self { score, item, seq }
+        Self {
+            score: OrderedFloat(score),
+            item,
+            seq,
+        }
+    }
+
+    pub fn get_score(&self) -> f64 {
+        self.score.0
+    }
+
+    pub fn into_bfs_item(self) -> BfsItem {
+        self.item
     }
 }
 
 impl PartialEq for ResultBucketItem {
     fn eq(&self, other: &Self) -> bool {
-        self.score == other.score && self.seq == other.seq
+        self.score.eq(&other.score) && self.seq.eq(&other.seq)
     }
 }
 
@@ -333,10 +122,9 @@ impl Ord for ResultBucketItem {
     fn cmp(&self, other: &Self) -> Ordering {
         // BinaryHeap is a max-heap: the greatest element is at the top.
         // We want the entry with the largest score at the top.
-        match self.score.partial_cmp(&other.score) {
+        self.score
-            Some(Ordering::Equal) | None => self.seq.cmp(&other.seq),
+            .cmp(&other.score)
-            Some(ord) => ord,
+            .then_with(|| self.seq.cmp(&other.seq))
-        }
     }
 }
 
@@ -357,11 +145,16 @@ pub struct ResultBucket {
 
 impl ResultBucket {
     /// Create a new bucket that holds at most `n` items.
-    pub fn new(n: usize) -> Self {
+    pub fn new(n: usize) -> Result<Self, BfsResolverError> {
-        Self {
+        // Check heap size
-            n,
+        if n == 0 {
-            heap: BinaryHeap::new(),
+            Err(BfsResolverError::BadBinHeapSize(n))
-            counter: 0,
+        } else {
+            Ok(Self {
+                n,
+                heap: BinaryHeap::new(),
+                counter: 0,
+            })
         }
     }
 
@@ -375,6 +168,11 @@ impl ResultBucket {
         self.heap.is_empty()
     }
 
+    /// Consume the bucket and return all stored items.
+    pub fn into_iter(self) -> impl Iterator<Item = BfsItem> {
+        self.heap.into_iter().map(|entry| entry.item)
+    }
+
     /// Insert a [`BfsItem`] with the given score.
     ///
     /// If the bucket is not yet full the item is always inserted.
@@ -382,33 +180,30 @@ impl ResultBucket {
     /// than the largest score currently in the bucket; the entry
     /// with the largest score is then evicted.
     ///
-    /// # Returns
+    /// Returns `true` if the item was inserted, `false` otherwise.
-    ///
-    /// `true` if the item was inserted, `false` otherwise.
     pub fn insert(&mut self, item: BfsItem, score: f64) -> bool {
         let entry = ResultBucketItem::new(score, item, self.counter);
         if self.heap.len() < self.n {
             self.heap.push(entry);
             self.counter += 1;
             true
-        } else if score >= self.heap.peek().unwrap().score {
+        } else if score
+            >= self
+                .heap
+                .peek()
+                .expect("unexpected blank binary heap")
+                .get_score()
+        {
             false
         } else {
-            *self.heap.peek_mut().unwrap() = entry;
+            *self.heap.peek_mut().expect("unexpected blank binary heap") = entry;
             self.counter += 1;
             true
         }
     }
-
-    /// Consume the bucket and return all stored items.
-    pub fn into_items(self) -> Vec<BfsItem> {
-        self.heap.into_iter().map(|entry| entry.item).collect()
-    }
 }
 
-// ============================================================================
+// endregion
-// BfsResolver
-// ============================================================================
 
 /// A resolver that uses brute-force search to find the best matching circuits.
 pub struct BfsResolver {
@@ -417,25 +212,72 @@ pub struct BfsResolver {
 }
 
 impl BfsResolver {
-    /// Create a new BFS resolver with the given datasets.
+    // YYC MARK:
-    pub fn new(datasets: DatasetCollection) -> Self {
+    // Some circuit are equivalent in topology.
-        Self { datasets }
+    // If we deduplicate these equaivalent circuit in building result,
-    }
+    // there are too complex works.
+    // So we should deduplicated these equivalent circuit at the beginning,
+    // i.e. when generating them.
+    // So following iterator structs are taking this job.
 
     /// Iterate all possible circuits with one device without repeating equivalent topology.
-    pub fn iter_one_device_circuit(dataset: &Dataset) -> OneDeviceCircuitIter<'_> {
+    pub fn iter_one_device_circuit(
-        OneDeviceCircuitIter::new(dataset.items())
+        dataset: &Dataset,
+    ) -> impl Iterator<Item = Result<Circuit, CircuitError>> {
+        // Every single device is unique so we directly output them.
+        // This feature is insured by dataset itself.
+        dataset.values().map(|v1| Circuit::from_one_device(v1))
     }
 
     /// Iterate all possible circuits with two devices without repeating equivalent topology.
-    pub fn iter_two_devices_circuit(dataset: &Dataset) -> TwoDeviceCircuitIter<'_> {
+    pub fn iter_two_devices_circuit(
-        TwoDeviceCircuitIter::new(dataset.items())
+        dataset: &Dataset,
+    ) -> impl Iterator<Item = Result<Circuit, CircuitError>> {
+        // The two devices in this circuit is always swapable,
+        // so we iterate them without repeating.
+        itertools::iproduct!(
+            dataset.values().array_combinations_with_replacement::<2>(),
+            JointKind::iter()
+        )
+        .map(|([v1, v2], j2)| Circuit::from_two_devices(v1, v2, j2))
     }
 
     /// Iterate all possible circuits with three devices without repeating equivalent topology.
-    pub fn iter_three_devices_circuit(dataset: &Dataset) -> ThreeDeviceCircuitIter<'_> {
+    pub fn iter_three_devices_circuit(
-        ThreeDeviceSameJointIter::new(dataset.items())
+        dataset: &Dataset,
-            .chain(ThreeDeviceDiffJointIter::new(dataset.items()))
+    ) -> impl Iterator<Item = Result<Circuit, CircuitError>> {
+        // For generating three devices circuit,
+        // it should be consisted by 2 parts.
+        itertools::chain!(
+            // First, the whole circuit has only one joint type.
+            // In this case, 3 devices are swapable and we should iterate them without repeating
+            itertools::iproduct!(
+                dataset.values().array_combinations_with_replacement::<3>(),
+                JointKind::iter()
+            )
+            .map(|([v1, v2, v3], j)| Circuit::from_three_devices(v1, v2, j, v3, j)),
+            // Second, if the joint type is different, then the first 2 devices are swapable.
+            // So we need iterate them without repeating.
+            itertools::iproduct!(
+                dataset.values().array_combinations_with_replacement::<2>(),
+                dataset.values(),
+                JointKind::iter()
+            )
+            .map(|([v1, v2], v3, j)| Circuit::from_three_devices(
+                v1,
+                v2,
+                j,
+                v3,
+                j.flip()
+            )),
+        )
+    }
+}
+
+impl BfsResolver {
+    /// Create a new BFS resolver with the given datasets.
+    pub fn new(datasets: DatasetCollection) -> Self {
+        Self { datasets }
     }
 
     fn pick_dataset(&self, device_kind: DeviceKind) -> &Dataset {
@@ -445,37 +287,47 @@ impl BfsResolver {
             DeviceKind::Inductor => self.datasets.inductor_dataset(),
         }
     }
-}
 
-impl Resolver for BfsResolver {
+    fn bfs_iteration(
-    fn resolve(&self, request: &Request) -> Result<Response, LcrConnError> {
+        dataset: &Dataset,
+        ccalc: &CircuitCalculator,
+    ) -> impl Iterator<Item = Result<BfsItem, BfsResolverError>> {
+        itertools::chain!(
+            BfsResolver::iter_one_device_circuit(&dataset),
+            BfsResolver::iter_two_devices_circuit(&dataset),
+            BfsResolver::iter_three_devices_circuit(&dataset)
+        )
+        .map(|circuit| -> Result<BfsItem, BfsResolverError> { BfsItem::new(circuit?, ccalc) })
+    }
+
+    fn intern_resolve(&self, request: &Request) -> Result<Response, BfsResolverError> {
         // Pick dataset from collection
-        let dataset = self.pick_dataset(request.device_kind);
+        let dataset = self.pick_dataset(request.get_device_kind());
 
         // Iterate circuit item one by one
-        let mut bucket = ResultBucket::new(request.count_limit);
+        let mut bucket = ResultBucket::new(request.get_count_limit())?;
-        let ccalc = CircuitCalculator::new(request.device_kind, request.target_value);
+        let ccalc = CircuitCalculator::new(request.get_device_kind(), request.get_target_value())?;
 
-        let circuits = Self::iter_one_device_circuit(dataset)
+        for item in BfsResolver::bfs_iteration(dataset, &ccalc) {
-            .chain(Self::iter_two_devices_circuit(dataset))
+            let item = item?;
-            .chain(Self::iter_three_devices_circuit(dataset));
-
-        for circuit in circuits {
-            let item = BfsItem::new(circuit, &ccalc)?;
             // If circuit absolute difference is out of tolerance, skip it directly.
-            if item.unsigned_difference() > request.tolerance {
+            if item.unsigned_difference() <= request.get_tolerance() {
+                // Put it into bucket
+                let score = item.unsigned_difference();
+                bucket.insert(item, score);
+            } else {
                 continue;
             }
-            // Put it into bucket
-            bucket.insert(item, item.unsigned_difference());
         }
 
         // Return result
-        let circuits: Vec<Circuit> = bucket
+        let circuits = bucket.into_iter().map(|i| i.into_circuit());
-            .into_items()
+        Ok(Response::new(request, circuits)?)
-            .into_iter()
+    }
-            .map(BfsItem::into_circuit)
+}
-            .collect();
+
-        Response::new(request, circuits)
+impl Resolver for BfsResolver {
+    fn resolve(&self, request: &Request) -> Result<Response, ResolverError> {
+        Ok(self.intern_resolve(request)?)
     }
 }

kernel/lcrconn/src/resolver/mod.rs

@@ -1,26 +0,0 @@
-pub mod bfs;
-pub mod lut;
-
-use crate::common::LcrConnError;
-use crate::query::{Request, Response};
-
-/// Abstract base trait for all resolvers.
-pub trait Resolver {
-    /// Resolve the request and return the response.
-    ///
-    /// # Arguments
-    ///
-    /// * `request` - The request to resolve.
-    ///
-    /// # Returns
-    ///
-    /// The response containing the best matching circuits.
-    ///
-    /// # Errors
-    ///
-    /// See [`Circuit::compute`](crate::common::Circuit::compute).
-    fn resolve(&self, request: &Request) -> Result<Response, LcrConnError>;
-}
-
-pub use bfs::BfsResolver;
-pub use lut::LutResolver;