From 3ed4f902cb04fb20a123ecbba2db7c05ba5289b4 Mon Sep 17 00:00:00 2001 From: James Tomlinson Date: Mon, 29 Jul 2024 22:33:10 +0100 Subject: [PATCH] feat: Unify solver features and fix Clippy issues. (#232) Fix several issues regarding features that Clippy raised. Extends Clippy coverage to all crates except `ipm-simd`, which only works on nightly. Unifies the solver features to `pywr-cli`, `pywr-schema` and `pywr-python` crates. These enable their dependent crate features as required. --- .github/workflows/linux.yml | 2 +- ipm-ocl/src/lib.rs | 23 ++++++----------------- pywr-cli/Cargo.toml | 6 ++++++ pywr-cli/src/main.rs | 10 +++++----- pywr-core/src/network.rs | 2 -- pywr-python/Cargo.toml | 6 ++++++ pywr-python/src/lib.rs | 14 +++++++++----- pywr-schema/Cargo.toml | 4 ++++ 8 files changed, 37 insertions(+), 30 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index d7a2cdb0..b9c09a95 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -20,7 +20,7 @@ jobs: with: submodules: true - name: Run Clippy - run: cargo clippy --all-targets --features highs,cbc + run: cargo clippy --all-targets --features highs,cbc --all --exclude ipm-simd - name: Install latest mdbook run: | tag=$(curl 'https://api.github.com/repos/rust-lang/mdbook/releases/latest' | jq -r '.tag_name') diff --git a/ipm-ocl/src/lib.rs b/ipm-ocl/src/lib.rs index d81843cf..81181746 100644 --- a/ipm-ocl/src/lib.rs +++ b/ipm-ocl/src/lib.rs @@ -108,26 +108,14 @@ where let row_offsets = ocl::Buffer::::builder() .queue(queue.clone()) .flags(ocl::flags::MEM_READ_WRITE) - .copy_host_slice( - a.row_offsets() - .into_iter() - .map(|r| *r as u32) - .collect::>() - .as_slice(), - ) + .copy_host_slice(a.row_offsets().iter().map(|r| *r as u32).collect::>().as_slice()) .len(a.row_offsets().len()) .build()?; let col_indices = ocl::Buffer::::builder() .queue(queue.clone()) .flags(ocl::flags::MEM_READ_WRITE) - .copy_host_slice( - a.col_indices() - .into_iter() - .map(|r| *r as u32) - .collect::>() - .as_slice(), - ) + .copy_host_slice(a.col_indices().iter().map(|r| *r as u32).collect::>().as_slice()) .len(a.col_indices().len()) .build()?; @@ -439,6 +427,7 @@ impl PathFollowingDirectClSolver where T: ocl::OclPrm + GetClProgram, { + #[allow(clippy::too_many_arguments)] pub fn from_data( queue: &ocl::Queue, program: &ocl::Program, @@ -456,7 +445,7 @@ where let buffers = PathFollowingDirectClBuffers::from_data(&a, num_lps, queue)?; let kernel_normal_init = ocl::Kernel::builder() - .program(&program) + .program(program) .name("normal_eqn_init") .queue(queue.clone()) .global_work_size(num_lps) @@ -470,7 +459,7 @@ where .build()?; let kernel_normal_eq_step = ocl::Kernel::builder() - .program(&program) + .program(program) .name("normal_eqn_step") .queue(queue.clone()) .global_work_size(num_lps) @@ -619,6 +608,6 @@ mod tests { let queue = ocl::Queue::new(&context, device, None).unwrap(); let a = test_matrx(); - let pf = PathFollowingDirectClBuffers::from_data(&a, 10, &queue).unwrap(); + let _pf = PathFollowingDirectClBuffers::from_data(&a, 10, &queue).unwrap(); } } diff --git a/pywr-cli/Cargo.toml b/pywr-cli/Cargo.toml index b62630a6..04ba6e0a 100644 --- a/pywr-cli/Cargo.toml +++ b/pywr-cli/Cargo.toml @@ -26,3 +26,9 @@ schemars = { workspace = true } pywr-core = { path = "../pywr-core" } pywr-schema = { path = "../pywr-schema" } + +[features] +cbc = ["pywr-core/cbc", "pywr-schema/cbc"] +highs = ["pywr-core/highs", "pywr-schema/highs"] +ipm-ocl = ["pywr-core/ipm-ocl", "pywr-schema/ipm-ocl"] +ipm-simd = ["pywr-core/ipm-simd", "pywr-schema/ipm-simd"] diff --git a/pywr-cli/src/main.rs b/pywr-cli/src/main.rs index 17492bd1..ecdd25f8 100644 --- a/pywr-cli/src/main.rs +++ b/pywr-cli/src/main.rs @@ -24,7 +24,7 @@ use std::path::{Path, PathBuf}; enum Solver { Clp, #[cfg(feature = "highs")] - HIGHS, + Highs, #[cfg(feature = "ipm-ocl")] CLIPMF32, #[cfg(feature = "ipm-ocl")] @@ -38,7 +38,7 @@ impl Display for Solver { match self { Solver::Clp => write!(f, "clp"), #[cfg(feature = "highs")] - Solver::HIGHS => write!(f, "highs"), + Solver::Highs => write!(f, "highs"), #[cfg(feature = "ipm-ocl")] Solver::CLIPMF32 => write!(f, "clipmf32"), #[cfg(feature = "ipm-ocl")] @@ -265,7 +265,7 @@ fn run(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path: Opti match *solver { Solver::Clp => model.run::(&ClpSolverSettings::default()), #[cfg(feature = "highs")] - Solver::HIGHS => model.run::(&HighsSolverSettings::default()), + Solver::Highs => model.run::(&HighsSolverSettings::default()), #[cfg(feature = "ipm-ocl")] Solver::CLIPMF32 => model.run_multi_scenario::(&ClIpmSolverSettings::default()), #[cfg(feature = "ipm-ocl")] @@ -287,7 +287,7 @@ fn run_multi(path: &Path, solver: &Solver, data_path: Option<&Path>, output_path match *solver { Solver::Clp => model.run::(&ClpSolverSettings::default()), #[cfg(feature = "highs")] - Solver::HIGHS => model.run::(&HighsSolverSettings::default()), + Solver::Highs => model.run::(&HighsSolverSettings::default()), #[cfg(feature = "ipm-ocl")] Solver::CLIPMF32 => model.run_multi_scenario::(&ClIpmSolverSettings::default()), #[cfg(feature = "ipm-ocl")] @@ -305,7 +305,7 @@ fn run_random(num_systems: usize, density: usize, num_scenarios: usize, solver: match *solver { Solver::Clp => model.run::(&ClpSolverSettings::default()), #[cfg(feature = "highs")] - Solver::HIGHS => model.run::(&HighsSolverSettings::default()), + Solver::Highs => model.run::(&HighsSolverSettings::default()), #[cfg(feature = "ipm-ocl")] Solver::CLIPMF32 => model.run_multi_scenario::(&ClIpmSolverSettings::default()), #[cfg(feature = "ipm-ocl")] diff --git a/pywr-core/src/network.rs b/pywr-core/src/network.rs index 4034241f..48492b9d 100644 --- a/pywr-core/src/network.rs +++ b/pywr-core/src/network.rs @@ -1648,8 +1648,6 @@ mod tests { use crate::parameters::{ActivationFunction, ControlCurveInterpolatedParameter, Parameter}; use crate::recorders::AssertionRecorder; use crate::scenario::{ScenarioDomain, ScenarioGroupCollection, ScenarioIndex}; - #[cfg(feature = "clipm")] - use crate::solvers::{ClIpmF64Solver, SimdIpmF64Solver}; use crate::solvers::{ClpSolver, ClpSolverSettings}; use crate::test_utils::{run_all_solvers, simple_model, simple_storage_model}; use float_cmp::assert_approx_eq; diff --git a/pywr-python/Cargo.toml b/pywr-python/Cargo.toml index c11520fa..4ca77314 100644 --- a/pywr-python/Cargo.toml +++ b/pywr-python/Cargo.toml @@ -30,3 +30,9 @@ pywr-schema = { path = "../pywr-schema" } [lib] name = "pywr" crate-type = ["cdylib"] + +[features] +cbc = ["pywr-core/cbc", "pywr-schema/cbc"] +highs = ["pywr-core/highs", "pywr-schema/highs"] +ipm-ocl = ["pywr-core/ipm-ocl", "pywr-schema/ipm-ocl"] +ipm-simd = ["pywr-core/ipm-simd", "pywr-schema/ipm-simd"] diff --git a/pywr-python/src/lib.rs b/pywr-python/src/lib.rs index 29dd3055..b79cda0f 100644 --- a/pywr-python/src/lib.rs +++ b/pywr-python/src/lib.rs @@ -14,7 +14,7 @@ use pyo3::types::{PyDict, PyType}; use pywr_core::solvers::{ClIpmF32Solver, ClIpmF64Solver, ClIpmSolverSettings}; use pywr_core::solvers::{ClpSolver, ClpSolverSettings, ClpSolverSettingsBuilder}; #[cfg(feature = "highs")] -use pywr_core::solvers::{HighsSolver, HighsSolverSettings, HighsSolverSettings, HighsSolverSettingsBuilde}; +use pywr_core::solvers::{HighsSolver, HighsSolverSettings, HighsSolverSettingsBuilder}; use pywr_schema::model::DateType; use std::fmt; use std::path::PathBuf; @@ -118,12 +118,16 @@ impl Model { #[cfg(feature = "highs")] "highs" => { let settings = build_highs_settings(solver_kwargs)?; - model.run::(&HighsSolverSettings::default())?; + self.model.run::(&settings)?; } #[cfg(feature = "ipm-ocl")] - "clipm-f32" => model.run_multi_scenario::(&ClIpmSolverSettings::default()), + "clipm-f32" => self + .model + .run_multi_scenario::(&ClIpmSolverSettings::default()), #[cfg(feature = "ipm-ocl")] - "clipm-f64" => model.run_multi_scenario::(&ClIpmSolverSettings::default()), + "clipm-f64" => self + .model + .run_multi_scenario::(&ClIpmSolverSettings::default()), _ => return Err(PyRuntimeError::new_err(format!("Unknown solver: {}", solver_name))), } @@ -163,7 +167,7 @@ fn build_clp_settings(kwargs: Option<&Bound<'_, PyDict>>) -> PyResult) -> PyResult { +fn build_highs_settings(kwargs: Option<&Bound<'_, PyDict>>) -> PyResult { let mut builder = HighsSolverSettingsBuilder::default(); if let Some(kwargs) = kwargs { diff --git a/pywr-schema/Cargo.toml b/pywr-schema/Cargo.toml index ffd07a11..396509a1 100644 --- a/pywr-schema/Cargo.toml +++ b/pywr-schema/Cargo.toml @@ -42,3 +42,7 @@ tempfile = "3.3.0" # Core feature requires additional dependencies core = ["dep:pywr-core", "dep:hdf5", "dep:csv", "dep:polars", "dep:pyo3", "dep:pyo3-polars", "dep:ndarray", "dep:tracing"] default = ["core"] +cbc = ["pywr-core/cbc"] +highs = ["pywr-core/highs"] +ipm-ocl = ["pywr-core/ipm-ocl"] +ipm-simd = ["pywr-core/ipm-simd"]