Add extra test, fix reciprocal allocation matrix creation

src/overhead_allocation.rs

@@ -243,7 +243,7 @@ where
     let mut overhead_ccs: Vec<String> = Vec::new();
     // overhead department -> total (summed limit to costs)
     let mut overhead_cc_totals: HashMap<String, f64> = HashMap::new();
-    // For each overhead area, get the cost centres in the area, and get all cost centres
+    // For each overhead area, get the cost centres in the area (overhead cost centres), and get all cost centres
     // that fit the limit to criteria for the area (skip any cases of overhead cc = other cc).
     // Then get the totals for the other ccs, by looking in the flat_department_costs, where the
     // allocation statistic matches the allocation statistic for this area
@@ -253,24 +253,15 @@ where
         let area_name = area.get("Name").unwrap();
         let allocation_statistic = area.get("AllocationStatistic").unwrap();
         let department_type: DepartmentType = DepartmentType::from(area.get("Type").unwrap());
-        let current_area_ccs = area_ccs.get(area_name);
 
-        if current_area_ccs.is_none() {
-            continue;
-        }
-        // Also skip if allocation statistic isn't found
-        // let allocation_statistic_found = allocation_statistics
-        //     .iter()
-        //     .find(|stat| stat.name == *allocation_statistic);
-        // if allocation_statistic_found.is_none() {
-        //     continue;
-        // }
-
-        let mut current_area_ccs = current_area_ccs.unwrap().clone();
-
-        //TODO: This isn't bad, however if it's too slow then consider performance improvements, such as summing totals now
-        // rather than calculating in the next step.
         if department_type == DepartmentType::Overhead {
+            let current_area_ccs = area_ccs.get(area_name);
+
+            if current_area_ccs.is_none() {
+                continue;
+            }
+            let mut current_area_ccs = current_area_ccs.unwrap().clone();
+
             overhead_ccs.append(&mut current_area_ccs);
             let overhead_ccs = area_ccs.get(area_name).unwrap();
             // TODO: This depends on the area limit criteria. For now just doing any limit criteria
@@ -377,6 +368,12 @@ where
         }
     }
 
+    // TODO: (Consider) We could actually cheat here and not use this matrix implementation at all (and thus be more
+    // memory efficient, but maybe slower)
+    // Since we know each operating department in an account will get the proportion of the total overhead amount relative
+    // according to its operating amount from the total amount of the overhead departments, we can just directly calculate
+    // these totals and do some simple multiplications (it does get trickier with multiple accounts, as the cost drivers
+    // are consistent across all accounts, but depend on the allocation statistic to determine which lines to pick from).
     let results = reciprocal_allocation_impl(
         allocation_rules,
         initial_account_costs
@@ -390,11 +387,14 @@ where
 
     for cost in results {
         for department in cost.summed_department_costs {
-            output.serialize(CsvCost {
+            // Any consumers should assume missing cc/account value was 0 (we already ignore overhead, as they all 0 out)
-                account: cost.account.clone(),
+            if department.value != 0. {
-                department: department.department,
+                output.serialize(CsvCost {
-                value: department.value,
+                    account: cost.account.clone(),
-            })?;
+                    department: department.department,
+                    value: department.value,
+                })?;
+            }
         }
     }
     Ok(())
@@ -404,7 +404,7 @@ fn split_allocation_statistic_range(
     allocation_statistic: &CsvAllocationStatistic,
     accounts_sorted: &Vec<String>,
 ) -> Vec<AllocationStatisticAccountRange> {
-    // TODO: This split needs to be more comprehensive so that we don't split between quotes
+    // TODO: This split needs to be more comprehensive so that we don't split between quotes, so use a regex
     let split = allocation_statistic.account_ranges.split(";");
     split
         .map(|split| {
@@ -533,7 +533,7 @@ fn do_solve_reciprocal<T: ReciprocalAllocationSolver>(
                 .get(&rule.to_department)
                 .unwrap();
             operating_overhead_mappings
-                [from_index * overhead_department_mappings.len() + to_index] = rule.percent;
+                [from_index * operating_department_mappings.len() + to_index] = rule.percent;
         }
     }
     let operating_overhead_mappings_mat: DMatrix<f64> = DMatrix::from_vec(
@@ -598,6 +598,7 @@ fn do_solve_reciprocal<T: ReciprocalAllocationSolver>(
 mod tests {
     use crate::reciprocal_allocation;
     use crate::AccountCost;
+    use crate::CsvCost;
     use crate::DepartmentType;
     use crate::OverheadAllocationRule;
     use crate::TotalDepartmentCost;
@@ -683,6 +684,23 @@ mod tests {
         assert_eq!(expected_final_allocations, result);
     }
 
+    #[test]
+    fn test_basic_real() {
+        let result = reciprocal_allocation(
+            csv::Reader::from_path("test_line.csv").unwrap(),
+            csv::Reader::from_path("test_account.csv").unwrap(),
+            csv::Reader::from_path("test_alloc_stat.csv").unwrap(),
+            csv::Reader::from_path("test_area.csv").unwrap(),
+            csv::Reader::from_path("test_costcentre.csv").unwrap(),
+            &mut csv::Writer::from_path("test_output_alloc_stat.csv").unwrap(),
+            true,
+            false,
+            true,
+            "E".to_owned(),
+        );
+        assert!(result.is_ok());
+    }
+
     #[test]
     fn test_real() {
         let result = reciprocal_allocation(