Repository: mlondschien/changeforest (language: rust)

Issue:
Add `OptimizerResult` to `BinarySegmentationTree`



Discussion / hints:

Fix patch:
diff --git a/hdcd-python/src/result.rs b/hdcd-python/src/result.rs
index 7039b0b..0170953 100644
--- a/hdcd-python/src/result.rs
+++ b/hdcd-python/src/result.rs
@@ -149,24 +149,38 @@ impl MyBinarySegmentationResult {
 
     #[getter]
     fn best_split(&self) -> Option<usize> {
-        self.result.best_split
+        self.result
+            .optimizer_result
+            .as_ref()
+            .map(|result| result.best_split)
     }
 
     #[getter]
     fn max_gain(&self) -> Option<f64> {
-        self.result.max_gain
+        self.result
+            .optimizer_result
+            .as_ref()
+            .map(|result| result.max_gain)
     }
 
     #[getter]
-    fn gain_results(&self) -> Option<Vec<MyGainResult>> {
-        self.result.gain_results.as_ref().map(|results| {
-            results
-                .iter()
-                .map(|result| MyGainResult {
-                    result: result.clone(),
-                })
-                .collect()
-        })
+    fn p_value(&self) -> Option<f64> {
+        self.result.model_selection_result.p_value
+    }
+
+    #[getter]
+    fn is_significant(&self) -> bool {
+        self.result.model_selection_result.is_significant
+    }
+
+    #[getter]
+    fn optimizer_result(&self) -> Option<MyOptimizerResult> {
+        self.result
+            .optimizer_result
+            .as_ref()
+            .map(|result| MyOptimizerResult {
+                result: result.clone(),
+            })
     }
 
     #[getter]
diff --git a/hdcd-r/R/display.R b/hdcd-r/R/display.R
index f4b39da..981700b 100644
--- a/hdcd-r/R/display.R
+++ b/hdcd-r/R/display.R
@@ -27,8 +27,8 @@ to_data_frame = function(node, parent_indent="", prefix="") {
             name=name,
             best_split=node$best_split,
             max_gain=node$max_gain,
-            p_value=node$model_selection_result$p_value,
-            is_significant=node$model_selection_result$is_significant
+            p_value=node$p_value,
+            is_significant=node$is_significant
         ),
         left,
         right
diff --git a/hdcd-r/R/hdcd.R b/hdcd-r/R/hdcd.R
index 8d72a0f..34ab990 100644
--- a/hdcd-r/R/hdcd.R
+++ b/hdcd-r/R/hdcd.R
@@ -6,7 +6,9 @@ BinarySegmentationResult = R6::R6Class(
         best_split = NULL,
         max_gain = NULL,
         model_selection_result = NULL,
-        gain_results = NULL,
+        p_value = NULL,
+        is_significant = NULL,
+        optimizer_result = NULL,
         segments = NULL,
         left = NULL,
         right = NULL,
@@ -17,7 +19,9 @@ BinarySegmentationResult = R6::R6Class(
             best_split = NULL,
             max_gain = NULL,
             model_selection_result = NULL,
-            gain_results = NULL,
+            p_value = NULL,
+            is_significant = NULL,
+            optimizer_result = NULL,
             segments = NULL,
             left = NULL,
             right = NULL
@@ -27,7 +31,9 @@ BinarySegmentationResult = R6::R6Class(
             self$best_split = best_split
             self$max_gain = max_gain
             self$model_selection_result = model_selection_result
-            self$gain_results = gain_results
+            self$optimizer_result = optimizer_result
+            self$p_value = p_value
+            self$is_significant = is_significant
             self$segments = segments
             self$left = left
             self$right = right
diff --git a/hdcd-r/src/rust/src/result.rs b/hdcd-r/src/rust/src/result.rs
index b68882e..e3d1fae 100644
--- a/hdcd-r/src/rust/src/result.rs
+++ b/hdcd-r/src/rust/src/result.rs
@@ -112,14 +112,11 @@ impl From<MyBinarySegmentationResult> for Robj {
             None => ().into(),
         };
 
-        let gain_results: Robj = match my_result.result.gain_results {
-            Some(results) => {
-                let robjs: Vec<Robj> = results
-                    .into_iter()
-                    .map(|gain_result| MyGainResult { gain_result }.into())
-                    .collect();
-                robjs.into()
+        let optimizer_result: Robj = match my_result.result.optimizer_result.as_ref() {
+            Some(optimizer_result) => MyOptimizerResult {
+                optimizer_result: optimizer_result.clone(),
             }
+            .into(),
             None => ().into(),
         };
 
@@ -135,17 +132,27 @@ impl From<MyBinarySegmentationResult> for Robj {
         };
 
         let model_selection_result: Robj = MyModelSelectionResult {
-            model_selection_result: my_result.result.model_selection_result,
+            model_selection_result: my_result.result.model_selection_result.clone(),
         }
         .into();
 
         List::from_values(&[
             r!(my_result.result.start as i32),
             r!(my_result.result.stop as i32),
-            r!(my_result.result.best_split.map(|u| u as i32)),
-            r!(my_result.result.max_gain),
+            r!(my_result
+                .result
+                .optimizer_result
+                .as_ref()
+                .map(|result| result.best_split as i32)),
+            r!(my_result
+                .result
+                .optimizer_result
+                .as_ref()
+                .map(|result| result.max_gain)),
+            r!(my_result.result.model_selection_result.p_value),
+            r!(my_result.result.model_selection_result.is_significant),
+            r!(optimizer_result),
             r!(model_selection_result),
-            r!(gain_results),
             r!(segments),
             r!(left),
             r!(right),
@@ -156,8 +163,10 @@ impl From<MyBinarySegmentationResult> for Robj {
             "stop",
             "best_split",
             "max_gain",
+            "p_value",
+            "is_significant",
+            "optimizer_result",
             "model_selection_result",
-            "gain_results",
             "segments",
             "left",
             "right",
diff --git a/src/binary_segmentation.rs b/src/binary_segmentation.rs
index cb3338c..5b0b788 100644
--- a/src/binary_segmentation.rs
+++ b/src/binary_segmentation.rs
@@ -1,4 +1,3 @@
-use crate::gain::GainResult;
 use crate::optimizer::OptimizerResult;
 use crate::{ModelSelectionResult, Segmentation};
 
@@ -6,8 +5,6 @@ pub struct BinarySegmentationTree {
     pub start: usize,
     pub stop: usize,
     pub n: usize,
-    pub split: Option<usize>,
-    pub max_gain: Option<f64>,
     pub model_selection_result: ModelSelectionResult,
     pub left: Option<Box<BinarySegmentationTree>>,
     pub right: Option<Box<BinarySegmentationTree>>,
@@ -20,8 +17,6 @@ impl BinarySegmentationTree {
             start: 0,
             stop: X.nrows(),
             n: X.nrows(),
-            split: None,
-            max_gain: None,
             model_selection_result: ModelSelectionResult::default(),
             left: None,
             right: None,
@@ -34,8 +29,6 @@ impl BinarySegmentationTree {
             start: self.start,
             stop: split,
             n: self.n,
-            split: None,
-            max_gain: None,
             model_selection_result: ModelSelectionResult::default(),
             left: None,
             right: None,
@@ -48,8 +41,6 @@ impl BinarySegmentationTree {
             start: split,
             stop: self.stop,
             n: self.n,
-            split: None,
-            max_gain: None,
             model_selection_result: ModelSelectionResult::default(),
             left: None,
             right: None,
@@ -57,7 +48,7 @@ impl BinarySegmentationTree {
         })
     }
 
-    /// Grow a `BinarySegmentationTree`.assert_approx_eq
+    /// Grow a `BinarySegmentationTree`.
     ///
     /// Recursively split segments and add subsegments as children `left` and
     /// `right` until segments are smaller then the minimal segment length
@@ -65,9 +56,6 @@ impl BinarySegmentationTree {
     /// longer significant.
     pub fn grow(&mut self, segmentation: &mut Segmentation) {
         if let Ok(optimizer_result) = segmentation.find_best_split(self.start, self.stop) {
-            self.split = Some(optimizer_result.best_split);
-            self.max_gain = Some(optimizer_result.max_gain);
-
             self.model_selection_result = segmentation.model_selection(&optimizer_result);
 
             if self.model_selection_result.is_significant {
@@ -90,10 +78,8 @@ impl BinarySegmentationTree {
 pub struct BinarySegmentationResult {
     pub start: usize,
     pub stop: usize,
-    pub best_split: Option<usize>,
-    pub max_gain: Option<f64>,
     pub model_selection_result: ModelSelectionResult,
-    pub gain_results: Option<Vec<GainResult>>,
+    pub optimizer_result: Option<OptimizerResult>,
     pub left: Option<Box<BinarySegmentationResult>>,
     pub right: Option<Box<BinarySegmentationResult>>,
     pub segments: Option<Vec<OptimizerResult>>,
@@ -109,15 +95,11 @@ impl BinarySegmentationResult {
             .right
             .map(|tree| Box::new(BinarySegmentationResult::from_tree(*tree)));
 
-        let gain_results = tree.optimizer_result.map(|result| result.gain_results);
-
         BinarySegmentationResult {
             start: tree.start,
             stop: tree.stop,
-            best_split: tree.split,
-            max_gain: tree.max_gain,
             model_selection_result: tree.model_selection_result,
-            gain_results,
+            optimizer_result: tree.optimizer_result,
             left,
             right,
             segments: None,
@@ -131,9 +113,9 @@ impl BinarySegmentationResult {
             split_points.append(&mut left_boxed.split_points());
         }
 
-        if let Some(best_split) = self.best_split {
+        if let Some(result) = self.optimizer_result.as_ref() {
             if self.model_selection_result.is_significant {
-                split_points.push(best_split);
+                split_points.push(result.best_split);
             }
         }
 
@@ -173,9 +155,7 @@ mod tests {
 
         binary_segmentation.grow(&mut segmentation);
 
-        assert_eq!(binary_segmentation.split, Some(25));
-
-        let optimizer_result = binary_segmentation.optimizer_result.unwrap();
+        let optimizer_result = binary_segmentation.optimizer_result.as_ref().unwrap();
         assert_eq!(optimizer_result.best_split, 25);
         assert_eq!(optimizer_result.start, 0);
         assert_eq!(optimizer_result.stop, 100);
@@ -200,25 +180,25 @@ mod tests {
         assert_eq!(result.split_points(), vec![25, 40, 80]);
         assert_eq!(result.start, 0);
         assert_eq!(result.stop, 100);
-        assert_eq!(result.best_split, Some(25));
+        assert_eq!(result.optimizer_result.as_ref().unwrap().best_split, 25);
         assert!(result.model_selection_result.is_significant);
-        assert!(result.gain_results.is_some());
+        assert!(result.optimizer_result.is_some());
 
         let right = result.right.as_ref().unwrap();
         assert_eq!(right.split_points(), vec![40, 80]);
         assert_eq!(right.start, 25);
         assert_eq!(right.stop, 100);
-        assert_eq!(right.best_split, Some(40));
+        assert_eq!(right.optimizer_result.as_ref().unwrap().best_split, 40);
         assert!(right.model_selection_result.is_significant);
-        assert!(right.gain_results.is_some());
+        assert!(right.optimizer_result.is_some());
 
         let left = result.left.as_ref().unwrap();
         assert_eq!(left.split_points(), vec![]);
         assert_eq!(left.start, 0);
         assert_eq!(left.stop, 25);
-        assert_eq!(left.best_split, Some(10));
+        assert_eq!(left.optimizer_result.as_ref().unwrap().best_split, 10);
         assert!(!left.model_selection_result.is_significant);
-        assert!(left.gain_results.is_some()); // even though is_significant is false
+        assert!(left.optimizer_result.is_some()); // even though is_significant is false
 
         let result = result.with_segments(segmentation);
         assert!(!result.segments.as_ref().unwrap().is_empty());


Test patch: