fix: fix request error

kernel/lcrconn/src/query.rs

@@ -1,10 +1,12 @@
-use std::cmp::Ordering;
+use crate::common::{
-use std::ops::Index;
+    Circuit, CircuitCalculator, CircuitCalculatorError, DeviceKind, DeviceValueError,
-
+    sanitize_device_value,
-use crate::common::{Circuit, CircuitCalculator, DeviceKind, LcrConnError};
+};
+use ordered_float::OrderedFloat;
+use thiserror::Error as TeError;
 
 /// The priority of the result.
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ResponsePriority {
     /// Less devices is the first priority.
     LessDevices,
@@ -15,46 +17,50 @@ pub enum ResponsePriority {
 /// The maximum count for the response item count passed in request.
 pub const MAX_RESPONSE_CNT: usize = 50;
 
+/// The error occurs when building [Request].
+#[derive(Debug, TeError)]
+pub enum RequestError {
+    #[error("invalid target value in request: {0}")]
+    BadTargetValue(DeviceValueError),
+    #[error("invalid tolerance in request: {0}")]
+    BadTolerance(DeviceValueError),
+    #[error("invalid response count {0} limit in request")]
+    BadCountLimit(usize),
+}
+
 /// All request information for the resolver.
 #[derive(Clone, Debug)]
 pub struct Request {
     /// The kind of device to resolve.
-    pub device_kind: DeviceKind,
+    device_kind: DeviceKind,
     /// The target value of the device.
-    pub target_value: f64,
+    target_value: f64,
     /// The tolerance of the device in absolute value.
-    pub tolerance: f64,
+    tolerance: f64,
     /// The priority principle when sorting response items.
-    pub response_priority: ResponsePriority,
+    response_priority: ResponsePriority,
     /// The limited count of results.
-    pub count_limit: usize,
+    count_limit: usize,
 }
 
 impl Request {
     /// Create a new request with validation.
-    ///
-    /// # Errors
-    ///
-    /// Returns [`LcrConnError::InvalidTargetValue`] if the target value is not greater than 0.
-    /// Returns [`LcrConnError::InvalidTolerance`] if the tolerance is negative.
-    /// Returns [`LcrConnError::InvalidCountLimit`] if the count limit is 0 or exceeds
-    /// [`MAX_RESPONSE_CNT`].
     pub fn new(
         device_kind: DeviceKind,
         target_value: f64,
         tolerance: f64,
         response_priority: ResponsePriority,
         count_limit: usize,
-    ) -> Result<Self, LcrConnError> {
+    ) -> Result<Self, RequestError> {
-        if target_value <= 0.0 {
+        // Check arguments
-            return Err(LcrConnError::InvalidTargetValue(target_value));
+        let target_value =
-        }
+            sanitize_device_value(target_value).map_err(|err| RequestError::BadTargetValue(err))?;
-        if tolerance < 0.0 {
+        let tolerance =
-            return Err(LcrConnError::InvalidTolerance(tolerance));
+            sanitize_device_value(tolerance).map_err(|err| RequestError::BadTolerance(err))?;
-        }
         if count_limit == 0 || count_limit > MAX_RESPONSE_CNT {
-            return Err(LcrConnError::InvalidCountLimit(count_limit));
+            return Err(RequestError::BadCountLimit(count_limit));
         }
+        // Everything is okey.
         Ok(Self {
             device_kind,
             target_value,
@@ -63,6 +69,38 @@ impl Request {
             count_limit,
         })
     }
+
+    /// Get the kind of device of this request.
+    pub fn get_device_kind(&self) -> DeviceKind {
+        self.device_kind
+    }
+
+    /// Get the target value of this request.
+    pub fn get_target_value(&self) -> f64 {
+        self.target_value
+    }
+
+    /// Get the tolerance of this request.
+    pub fn get_tolerance(&self) -> f64 {
+        self.tolerance
+    }
+
+    /// Get the priority principle when sorting response items.
+    pub fn get_response_priority(&self) -> ResponsePriority {
+        self.response_priority
+    }
+
+    /// Get the limited count of results.
+    pub fn get_count_limit(&self) -> usize {
+        self.count_limit
+    }
+}
+
+/// Error occurs when building [Response] and [ResponseItem].
+#[derive(Debug, TeError)]
+pub enum ResponseError {
+    #[error("failed on computing circuit properties: {0}")]
+    CircuitCalculator(#[from] CircuitCalculatorError),
 }
 
 /// The possible solution given by the resolver.
@@ -70,44 +108,34 @@ impl Request {
 pub struct ResponseItem {
     /// The circuit of this response item.
     circuit: Circuit,
-    /// The device count of this circuit.
-    device_count: usize,
     /// The value of this circuit.
     value: f64,
-    /// The signed difference between the target value and the value of this circuit.
+    /// The signed difference.
-    ///
-    /// Positive value indicates that the value of this circuit is greater than the target value.
-    /// Negative value indicates that the value of this circuit is less than the target value.
     difference: f64,
-    /// The unsigned difference between the target value and the value of this circuit.
+    /// The unsigned difference.
     unsigned_difference: f64,
-    /// The signed relative difference between the target value and the value of this circuit.
+    /// The signed relative difference.
-    ///
-    /// Positive value indicates that the value of this circuit is greater than the target value.
-    /// Negative value indicates that the value of this circuit is less than the target value.
     relative_difference: f64,
-    /// The unsigned relative difference between the target value and the value of this circuit.
+    /// The unsigned relative difference.
     unsigned_relative_difference: f64,
 }
 
 impl ResponseItem {
     /// Create a new response item by computing all values eagerly.
-    ///
+    pub fn new(circuit: Circuit, ccalc: &CircuitCalculator) -> Result<Self, ResponseError> {
-    /// # Errors
+        // YYC MARK:
-    ///
+        // I can use OnceLock to implement the behavior closing to Python cached_property.
-    /// See [`CircuitValueTrait::value`].
+        // But I didn't do that due to the increased size of this struct, and inviable error handling.
-    pub fn new(circuit: Circuit, cv_trait: &CircuitCalculator) -> Result<Self, LcrConnError> {
+        // So I decide to calculate all values in there.
-        let value = cv_trait.value(&circuit)?;
+        let value = ccalc.value(&circuit)?;
-        let difference = cv_trait.difference(&circuit, Some(value))?;
+        let difference = ccalc.difference(&circuit, Some(value))?;
-        let unsigned_difference = cv_trait.unsigned_difference(&circuit, None, Some(difference))?;
+        let unsigned_difference = ccalc.unsigned_difference(&circuit, None, Some(difference))?;
-        let relative_difference = cv_trait.relative_difference(&circuit, None, Some(difference))?;
+        let relative_difference = ccalc.relative_difference(&circuit, None, Some(difference))?;
         let unsigned_relative_difference =
-            cv_trait.unsigned_relative_difference(&circuit, None, None, Some(relative_difference))?;
+            ccalc.unsigned_relative_difference(&circuit, None, None, Some(relative_difference))?;
-        let device_count = circuit.device_scale().to_device_count();
 
         Ok(Self {
             circuit,
-            device_count,
             value,
             difference,
             unsigned_difference,
@@ -123,7 +151,7 @@ impl ResponseItem {
 
     /// The device count of this circuit.
     pub fn device_count(&self) -> usize {
-        self.device_count
+        self.circuit.device_scale().to_device_count()
     }
 
     /// The value of this circuit.
@@ -179,35 +207,29 @@ impl Response {
     /// # Errors
     ///
     /// See [`ResponseItem::new`].
-    pub fn new(
+    pub fn new<I>(request: &Request, candidates: I) -> Result<Self, ResponseError>
-        request: &Request,
+    where
-        candidates: impl IntoIterator<Item = Circuit>,
+        I: Iterator<Item = Circuit>,
-    ) -> Result<Self, LcrConnError> {
+    {
-        let cv_trait = CircuitCalculator::new(request.device_kind, request.target_value);
+        let ccalc = CircuitCalculator::new(request.device_kind, request.target_value)?;
-
         let mut items: Vec<ResponseItem> = candidates
             .into_iter()
-            .map(|c| ResponseItem::new(c, &cv_trait))
+            .map(|c| ResponseItem::new(c, &ccalc))
             .collect::<Result<_, _>>()?;
 
         // Sort by different strategy
         match request.response_priority {
             ResponsePriority::LessDevices => {
                 items.sort_by(|a, b| {
-                    a.device_count
+                    a.device_count().cmp(&b.device_count()).then_with(|| {
-                        .cmp(&b.device_count)
+                        OrderedFloat(a.unsigned_difference)
-                        .then_with(|| {
+                            .cmp(&OrderedFloat(b.unsigned_difference))
-                            a.unsigned_difference
-                                .partial_cmp(&b.unsigned_difference)
-                                .unwrap_or(Ordering::Equal)
                     })
                 });
             }
             ResponsePriority::MoreAccuracy => {
                 items.sort_by(|a, b| {
-                    a.unsigned_difference
+                    OrderedFloat(a.unsigned_difference).cmp(&OrderedFloat(b.unsigned_difference))
-                        .partial_cmp(&b.unsigned_difference)
-                        .unwrap_or(Ordering::Equal)
                 });
             }
         }
@@ -246,11 +268,3 @@ impl Response {
         self.sorted_items.iter()
     }
 }
-
-impl Index<usize> for Response {
-    type Output = ResponseItem;
-
-    fn index(&self, index: usize) -> &Self::Output {
-        &self.sorted_items[index]
-    }
-}

kernel/lcrconn/src/resolver/bfs.rs

@@ -263,9 +263,9 @@ impl BfsItem {
     /// # Errors
     ///
     /// See [`CircuitValueTrait::value`].
-    pub fn new(circuit: Circuit, cv_trait: &CircuitCalculator) -> Result<Self, LcrConnError> {
+    pub fn new(circuit: Circuit, ccalc: &CircuitCalculator) -> Result<Self, LcrConnError> {
-        let value = cv_trait.value(&circuit)?;
+        let value = ccalc.value(&circuit)?;
-        let unsigned_difference = cv_trait.unsigned_difference(&circuit, Some(value))?;
+        let unsigned_difference = ccalc.unsigned_difference(&circuit, Some(value))?;
         Ok(Self {
             circuit,
             value,
@@ -454,14 +454,14 @@ impl Resolver for BfsResolver {
 
         // Iterate circuit item one by one
         let mut bucket = ResultBucket::new(request.count_limit);
-        let cv_trait = CircuitCalculator::new(request.device_kind, request.target_value);
+        let ccalc = CircuitCalculator::new(request.device_kind, request.target_value);
 
         let circuits = Self::iter_one_device_circuit(dataset)
             .chain(Self::iter_two_devices_circuit(dataset))
             .chain(Self::iter_three_devices_circuit(dataset));
 
         for circuit in circuits {
-            let item = BfsItem::new(circuit, &cv_trait)?;
+            let item = BfsItem::new(circuit, &ccalc)?;
             // If circuit absolute difference is out of tolerance, skip it directly.
             if item.unsigned_difference() > request.tolerance {
                 continue;

kernel/lcrconn/src/resolver/lut.rs

@@ -104,7 +104,7 @@ impl Resolver for LutResolver {
         let mut right = idx as isize;
         let lut_len = lut.len() as isize;
 
-        let cv_trait = CircuitCalculator::new(request.device_kind, target);
+        let ccalc = CircuitCalculator::new(request.device_kind, target);
 
         while left >= 0 || right < lut_len {
             if bucket.len() >= count_limit {
@@ -118,9 +118,9 @@ impl Resolver for LutResolver {
             } else {
                 let left_item = &lut[left as usize];
                 let left_diff =
-                    cv_trait.unsigned_difference(left_item.circuit(), Some(left_item.value()))?;
+                    ccalc.unsigned_difference(left_item.circuit(), Some(left_item.value()))?;
                 let right_item = &lut[right as usize];
-                let right_diff = cv_trait
+                let right_diff = ccalc
                     .unsigned_difference(right_item.circuit(), Some(right_item.value()))?;
                 left_diff <= right_diff
             };
@@ -135,7 +135,7 @@ impl Resolver for LutResolver {
                 item
             };
 
-            let diff = cv_trait.unsigned_difference(item.circuit(), Some(item.value()))?;
+            let diff = ccalc.unsigned_difference(item.circuit(), Some(item.value()))?;
             // Since the LUT is sorted, values on each side only move further
             // from target as we advance.  Once one side exceeds tolerance,
             // the rest of that side is guaranteed out of range — disable it.