From d266650d6f0d1ab942759d54ed32127e872bb661 Mon Sep 17 00:00:00 2001 From: James Tomlinson Date: Mon, 16 Dec 2024 14:54:45 +0000 Subject: [PATCH] fix: Properly skip failing IPM tests. --- pywr-core/src/aggregated_node.rs | 8 +-- pywr-core/src/network.rs | 6 +- pywr-core/src/recorders/mod.rs | 2 +- pywr-core/src/solvers/ipm_simd/settings.rs | 12 ++-- pywr-core/src/test_utils.rs | 37 +++++++++--- pywr-core/src/virtual_storage.rs | 6 +- pywr-schema/src/model.rs | 2 +- pywr-schema/tests/test_models.rs | 67 ++++++++++++---------- 8 files changed, 84 insertions(+), 56 deletions(-) diff --git a/pywr-core/src/aggregated_node.rs b/pywr-core/src/aggregated_node.rs index 64c848b2..6123ccf3 100644 --- a/pywr-core/src/aggregated_node.rs +++ b/pywr-core/src/aggregated_node.rs @@ -631,7 +631,7 @@ mod tests { let model = Model::new(default_time_domain().into(), network); - run_all_solvers(&model, &["ipm-simd", "ipm-ocl"], &[]); + run_all_solvers(&model, &["ipm-simd", "ipm-ocl"], &[], &[]); } /// Test the factors forcing a simple ratio of flow that varies over time @@ -685,7 +685,7 @@ mod tests { let model = Model::new(default_time_domain().into(), network); - run_all_solvers(&model, &["cbc", "ipm-simd", "ipm-ocl"], &[]); + run_all_solvers(&model, &["cbc", "ipm-simd", "ipm-ocl"], &[], &[]); } /// Test mutual exclusive flows @@ -741,7 +741,7 @@ mod tests { let model = Model::new(default_time_domain().into(), network); - run_all_solvers(&model, &["clp", "ipm-simd", "ipm-ocl"], &[]); + run_all_solvers(&model, &["clp", "ipm-simd", "ipm-ocl"], &[], &[]); } /// Test double mutual exclusive flows @@ -822,6 +822,6 @@ mod tests { let model = Model::new(default_time_domain().into(), network); - run_all_solvers(&model, &["clp", "ipm-ocl", "ipm-simd"], &[]); + run_all_solvers(&model, &["clp", "ipm-ocl", "ipm-simd"], &[], &[]); } } diff --git a/pywr-core/src/network.rs b/pywr-core/src/network.rs index c1054e8f..36f465ea 100644 --- a/pywr-core/src/network.rs +++ b/pywr-core/src/network.rs @@ -1814,7 +1814,7 @@ mod tests { model.network_mut().add_recorder(Box::new(recorder)).unwrap(); // Test all solvers - run_all_solvers(&model, &[], &[]); + run_all_solvers(&model, &[], &[], &[]); } #[test] @@ -1838,7 +1838,7 @@ mod tests { network.add_recorder(Box::new(recorder)).unwrap(); // Test all solvers - run_all_solvers(&model, &[], &[]); + run_all_solvers(&model, &[], &[], &[]); } /// Test proportional storage derived metric. @@ -1878,7 +1878,7 @@ mod tests { network.add_recorder(Box::new(recorder)).unwrap(); // Test all solvers - run_all_solvers(&model, &[], &[]); + run_all_solvers(&model, &[], &[], &[]); } #[test] diff --git a/pywr-core/src/recorders/mod.rs b/pywr-core/src/recorders/mod.rs index f3c26616..53b00f32 100644 --- a/pywr-core/src/recorders/mod.rs +++ b/pywr-core/src/recorders/mod.rs @@ -362,7 +362,7 @@ mod tests { let _idx = model.network_mut().add_recorder(Box::new(rec)).unwrap(); // Test all solvers - run_all_solvers(&model, &[], &[]); + run_all_solvers(&model, &[], &[], &[]); // TODO fix this with respect to the trait. // let array = rec.data_view2().unwrap(); diff --git a/pywr-core/src/solvers/ipm_simd/settings.rs b/pywr-core/src/solvers/ipm_simd/settings.rs index 2dae0fa4..0e9b7e51 100644 --- a/pywr-core/src/solvers/ipm_simd/settings.rs +++ b/pywr-core/src/solvers/ipm_simd/settings.rs @@ -67,17 +67,17 @@ where /// # Examples /// /// ``` -/// use std::num::NonZeroUsize; -/// use pywr::solvers::SimdIpmSolverSettingsBuilder; +/// use std::num::NonZero; +/// use pywr_core::solvers::{SimdIpmSolverSettings, SimdIpmSolverSettingsBuilder}; /// // Settings with parallel enabled and 4 threads. -/// let settings = SimdIpmSolverSettingsBuilder::default().parallel().threads(4).build(); +/// let settings: SimdIpmSolverSettings = SimdIpmSolverSettingsBuilder::default().parallel().threads(4).build(); /// /// let mut builder = SimdIpmSolverSettingsBuilder::default(); -/// builder.max_iterations(NonZeroUsize(50).unwrap()); -/// let settings = builder.build(); +/// builder.max_iterations(NonZero::new(50).unwrap()); +/// let settings: SimdIpmSolverSettings = builder.build(); /// /// builder.parallel(); -/// let settings = builder.build(); +/// let settings: SimdIpmSolverSettings = builder.build(); /// /// ``` pub struct SimdIpmSolverSettingsBuilder diff --git a/pywr-core/src/test_utils.rs b/pywr-core/src/test_utils.rs index 242f63fd..8864c958 100644 --- a/pywr-core/src/test_utils.rs +++ b/pywr-core/src/test_utils.rs @@ -166,7 +166,7 @@ pub fn run_and_assert_parameter( let rec = AssertionRecorder::new("assert", p_idx.into(), expected_values, ulps, epsilon); model.network_mut().add_recorder(Box::new(rec)).unwrap(); - run_all_solvers(model, &[], &[]) + run_all_solvers(model, &[], &[], &[]) } /// A struct to hold the expected outputs for a test. @@ -198,24 +198,47 @@ impl ExpectedOutputs { /// /// The model will only be run if the solver has the required solver features (and /// is also enabled as a Cargo feature). -pub fn run_all_solvers(model: &Model, solvers_without_features: &[&str], expected_outputs: &[ExpectedOutputs]) { - check_features_and_run::(model, !solvers_without_features.contains(&"clp"), expected_outputs); +pub fn run_all_solvers( + model: &Model, + solvers_without_features: &[&str], + solvers_to_skip: &[&str], + expected_outputs: &[ExpectedOutputs], +) { + if !solvers_to_skip.contains(&"clp") { + check_features_and_run::(model, !solvers_without_features.contains(&"clp"), expected_outputs); + } #[cfg(feature = "cbc")] { - check_features_and_run::(model, !solvers_without_features.contains(&"cbc"), expected_outputs); + if !solvers_to_skip.contains(&"cbc") { + check_features_and_run::(model, !solvers_without_features.contains(&"cbc"), expected_outputs); + } } #[cfg(feature = "highs")] { - check_features_and_run::(model, !solvers_without_features.contains(&"highs"), expected_outputs); + if !solvers_to_skip.contains(&"highs") { + check_features_and_run::( + model, + !solvers_without_features.contains(&"highs"), + expected_outputs, + ); + } } #[cfg(feature = "ipm-simd")] - check_features_and_run_multi::>(model, !solvers_without_features.contains(&"ipm-simd")); + { + if !solvers_to_skip.contains(&"ipm-simd") { + check_features_and_run_multi::>(model, !solvers_without_features.contains(&"ipm-simd")); + } + } #[cfg(feature = "ipm-ocl")] - check_features_and_run_multi::(model, !solvers_without_features.contains(&"ipm-ocl")); + { + if !solvers_to_skip.contains(&"ipm-ocl") { + check_features_and_run_multi::(model, !solvers_without_features.contains(&"ipm-ocl")); + } + } } /// Check features and diff --git a/pywr-core/src/virtual_storage.rs b/pywr-core/src/virtual_storage.rs index cb1cad9c..f2883081 100644 --- a/pywr-core/src/virtual_storage.rs +++ b/pywr-core/src/virtual_storage.rs @@ -427,7 +427,7 @@ mod tests { let domain = default_timestepper().try_into().unwrap(); let model = Model::new(domain, network); // Test all solvers - run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[]); + run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[], &[]); } #[test] @@ -454,7 +454,7 @@ mod tests { network.add_recorder(Box::new(recorder)).unwrap(); // Test all solvers - run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[]); + run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[], &[]); } #[test] @@ -494,6 +494,6 @@ mod tests { network.add_recorder(Box::new(recorder)).unwrap(); // Test all solvers - run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[]); + run_all_solvers(&model, &["ipm-ocl", "ipm-simd"], &[], &[]); } } diff --git a/pywr-schema/src/model.rs b/pywr-schema/src/model.rs index 2bbebcc3..1bd84a3a 100644 --- a/pywr-schema/src/model.rs +++ b/pywr-schema/src/model.rs @@ -1048,7 +1048,7 @@ mod core_tests { network.add_recorder(Box::new(rec)).unwrap(); // Test all solvers - run_all_solvers(&model, &[], &[]); + run_all_solvers(&model, &[], &[], &[]); } /// Test that a cycle in parameter dependencies does not load. diff --git a/pywr-schema/tests/test_models.rs b/pywr-schema/tests/test_models.rs index 5af032f4..597b620a 100644 --- a/pywr-schema/tests/test_models.rs +++ b/pywr-schema/tests/test_models.rs @@ -17,11 +17,11 @@ macro_rules! model_tests { // Deserialise the schema and run it #[cfg(feature = "core")] { - let (input, expected, solvers_without_features): (&str, Vec<&str>, Vec<&str>) = $value; + let (input, expected, solvers_without_features, solvers_to_skip): (&str, Vec<&str>, Vec<&str>, Vec<&str>) = $value; let input_pth = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests").join(input); let expected_paths = expected.iter().map(|p| Path::new(env!("CARGO_MANIFEST_DIR")).join("tests").join(p)).collect::>(); let schema = deserialise_test_model(&input_pth); - run_test_model(&schema, &expected_paths, &solvers_without_features); + run_test_model(&schema, &expected_paths, &solvers_without_features, &solvers_to_skip); } // Just deserialise the schema @@ -37,36 +37,36 @@ macro_rules! model_tests { } model_tests! { - test_simple1: ("simple1.json", vec![], vec![]), - test_csv1: ("csv1.json", vec!["csv1-outputs-long.csv", "csv1-outputs-wide.csv"], vec![]), - test_csv2: ("csv2.json", vec!["csv2-outputs-long.csv", "csv2-outputs-wide.csv"], vec![]), - test_csv3: ("csv3.json", vec!["csv3-outputs-long.csv"], vec![]), - test_hdf1: ("hdf1.json", vec![], vec![]), // TODO asserting h5 results not possible with this framework - test_memory1: ("memory1.json", vec![], vec![]), // TODO asserting memory results not possible with this framework - test_timeseries: ("timeseries.json", vec!["timeseries-expected.csv"], vec![]), - test_storage_max_volumes: ("storage_max_volumes.json", vec![], vec![]), - test_mutual_exclusivity1: ("mutual-exclusivity1.json", vec!["mutual-exclusivity1.csv"], vec!["clp", "ipm-simd", "ipm-ocl"]), - test_mutual_exclusivity2: ("mutual-exclusivity2.json", vec!["mutual-exclusivity2.csv"], vec!["clp", "ipm-simd", "ipm-ocl"]), - test_mutual_exclusivity3: ("mutual-exclusivity3.json", vec!["mutual-exclusivity3.csv"], vec!["clp", "ipm-simd", "ipm-ocl"]), - test_link_with_soft_min: ("link_with_soft_min.json", vec![], vec!["ipm-simd", "ipm-ocl"]), - test_link_with_soft_max: ("link_with_soft_max.json", vec![], vec!["ipm-simd", "ipm-ocl"]), - test_delay1: ("delay1.json", vec!["delay1-expected.csv"], vec![]), - test_loss_link1: ("loss_link1.json", vec!["loss_link1-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), - test_loss_link2: ("loss_link2.json", vec!["loss_link2-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), + test_simple1: ("simple1.json", vec![], vec![], vec![]), + test_csv1: ("csv1.json", vec!["csv1-outputs-long.csv", "csv1-outputs-wide.csv"], vec![], vec![]), + test_csv2: ("csv2.json", vec!["csv2-outputs-long.csv", "csv2-outputs-wide.csv"], vec![], vec![]), + test_csv3: ("csv3.json", vec!["csv3-outputs-long.csv"], vec![], vec![]), + test_hdf1: ("hdf1.json", vec![], vec![], vec![]), // TODO asserting h5 results not possible with this framework + test_memory1: ("memory1.json", vec![], vec![], vec![]), // TODO asserting memory results not possible with this framework + test_timeseries: ("timeseries.json", vec!["timeseries-expected.csv"], vec![], vec![]), + test_storage_max_volumes: ("storage_max_volumes.json", vec![], vec![], vec![]), + test_mutual_exclusivity1: ("mutual-exclusivity1.json", vec!["mutual-exclusivity1.csv"], vec!["clp", "ipm-simd", "ipm-ocl"], vec![]), + test_mutual_exclusivity2: ("mutual-exclusivity2.json", vec!["mutual-exclusivity2.csv"], vec!["clp", "ipm-simd", "ipm-ocl"], vec![]), + test_mutual_exclusivity3: ("mutual-exclusivity3.json", vec!["mutual-exclusivity3.csv"], vec!["clp", "ipm-simd", "ipm-ocl"], vec![]), + test_link_with_soft_min: ("link_with_soft_min.json", vec![], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_link_with_soft_max: ("link_with_soft_max.json", vec![], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_delay1: ("delay1.json", vec!["delay1-expected.csv"], vec![], vec![]), + test_loss_link1: ("loss_link1.json", vec!["loss_link1-expected.csv"], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_loss_link2: ("loss_link2.json", vec!["loss_link2-expected.csv"], vec!["ipm-simd", "ipm-ocl"], vec![]), // TODO this asserted internal flows in the previous test - test_piecewise_link1: ("piecewise_link1.json", vec!["piecewise-link1-nodes.csv", "piecewise-link1-edges.csv"], vec![]), + test_piecewise_link1: ("piecewise_link1.json", vec!["piecewise-link1-nodes.csv", "piecewise-link1-edges.csv"], vec![], vec![]), // TODO not sure why this is failing in IPM solvers (https://github.com/pywr/pywr-next/issues/293) - test_piecewise_storage1: ("piecewise_storage1.json", vec!["piecewise_storage1-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), + test_piecewise_storage1: ("piecewise_storage1.json", vec!["piecewise_storage1-expected.csv"], vec![], vec!["ipm-simd", "ipm-ocl"]), // TODO not sure why this is failing in IPM solvers (https://github.com/pywr/pywr-next/issues/293) - test_piecewise_storage2: ("piecewise_storage2.json", vec!["piecewise_storage2-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), - test_river_loss1: ("river_loss1.json", vec!["river_loss1-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), + test_piecewise_storage2: ("piecewise_storage2.json", vec!["piecewise_storage2-expected.csv"], vec![], vec!["ipm-simd", "ipm-ocl"]), + test_river_loss1: ("river_loss1.json", vec!["river_loss1-expected.csv"], vec!["ipm-simd", "ipm-ocl"], vec![]), // TODO not sure why this is failing in IPM solvers (https://github.com/pywr/pywr-next/issues/293) - test_river_gauge1: ("river_gauge1.json", vec![], vec!["ipm-simd", "ipm-ocl"]), - test_river_split_with_gauge1: ("river_split_with_gauge1.json", vec![], vec![]), - test_thirty_day_licence: ("30-day-licence.json", vec![], vec!["ipm-simd", "ipm-ocl"]), - test_wtw1: ("wtw1.json", vec!["wtw1-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), - test_wtw2: ("wtw2.json", vec!["wtw2-expected.csv"], vec!["ipm-simd", "ipm-ocl"]), - test_local_parameter1: ("local-parameter1.json", vec!["local-parameter1-expected.csv"], vec![]), + test_river_gauge1: ("river_gauge1.json", vec![], vec![], vec!["ipm-simd", "ipm-ocl"]), + test_river_split_with_gauge1: ("river_split_with_gauge1.json", vec![], vec![], vec![]), + test_thirty_day_licence: ("30-day-licence.json", vec![], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_wtw1: ("wtw1.json", vec!["wtw1-expected.csv"], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_wtw2: ("wtw2.json", vec!["wtw2-expected.csv"], vec!["ipm-simd", "ipm-ocl"], vec![]), + test_local_parameter1: ("local-parameter1.json", vec!["local-parameter1-expected.csv"], vec![], vec![]), } /// Test Pandas backend for reading timeseries data. @@ -82,7 +82,7 @@ fn test_timeseries_pandas() { .map(|p| Path::new(env!("CARGO_MANIFEST_DIR")).join("tests").join(p)) .collect::>(); let schema = deserialise_test_model(&input_pth); - run_test_model(&schema, &expected_paths, &[]); + run_test_model(&schema, &expected_paths, &[], &[]); } fn deserialise_test_model(model_path: &Path) -> PywrModel { @@ -91,7 +91,12 @@ fn deserialise_test_model(model_path: &Path) -> PywrModel { } #[cfg(feature = "core")] -fn run_test_model(schema: &PywrModel, result_paths: &[PathBuf], solvers_without_features: &[&str]) { +fn run_test_model( + schema: &PywrModel, + result_paths: &[PathBuf], + solvers_without_features: &[&str], + solvers_to_skip: &[&str], +) { let temp_dir = TempDir::new().unwrap(); let data_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests"); let model = schema.build_model(Some(&data_dir), Some(temp_dir.path())).unwrap(); @@ -107,7 +112,7 @@ fn run_test_model(schema: &PywrModel, result_paths: &[PathBuf], solvers_without_ .collect(); // Test all solvers - run_all_solvers(&model, solvers_without_features, &expected_outputs); + run_all_solvers(&model, solvers_without_features, solvers_to_skip, &expected_outputs); } macro_rules! convert_tests {