Skip to content

Commit

Permalink
Merge pull request #233 from alphaville/fix/227
Browse files Browse the repository at this point in the history
Clippy warnings
  • Loading branch information
alphaville authored Oct 27, 2021
2 parents bb63a80 + 2739c58 commit 6580b17
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 47 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ Note: This is the main Changelog file for the Rust solver. The Changelog file fo
<!-- ---------------------
Unreleased
--------------------- -->
## [Unreleased]

### Changed

* Removed unnecessary #[no_mangle] annotations
* Took care of additional clippy warnings


<!-- ---------------------
Expand Down Expand Up @@ -179,7 +185,7 @@ This is a breaking API change.
--------------------- -->

<!-- Releases -->
[Unreleased]: https://github.com/alphaville/optimization-engine/compare/v0.7.1...master
[Unreleased]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.5...master
[v0.7.1]: https://github.com/alphaville/optimization-engine/compare/v0.7.0...v0.7.1
[v0.7.0]: https://github.com/alphaville/optimization-engine/compare/v0.6.2...v0.7.0
[v0.6.2]: https://github.com/alphaville/optimization-engine/compare/v0.6.1-alpha.2...v0.6.2
Expand Down
36 changes: 30 additions & 6 deletions ci/script.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
#!/bin/bash
set -euxo pipefail

function run_clippy_test() {
cd $1
cargo clippy --all-targets --all-features
if [ -d "./tcp_iface_$1" ]
then
# Test auto-generated TCP interface
cd ./tcp_iface_$1
cargo clippy --all-targets --all-features
cd ..
fi
if [ -d "./tcp_iface_$1" ]
then
# Test auto-generated CasADi interface
cd icasadi_$1
cargo clippy --all-targets --all-features
cd ..
fi
cd ..
}

regular_test() {
# Run Python tests
# ------------------------------------
Expand Down Expand Up @@ -36,12 +56,16 @@ regular_test() {

# Run Clippy for generated optimizers
# ------------------------------------
cd .python_test_build/only_f1/tcp_iface_only_f1
cargo clippy --all-targets --all-features
cd ../../only_f2/tcp_iface_only_f2
cargo clippy --all-targets --all-features
cd ../../rosenbrock_ros
cargo clippy --all-targets --all-features

cd .python_test_build
run_clippy_test "only_f1"
run_clippy_test "only_f2"
run_clippy_test "halfspace_optimizer"
run_clippy_test "parametric_f2"
run_clippy_test "plain"
run_clippy_test "python_bindings"
run_clippy_test "rosenbrock_ros"

}

test_docker() {
Expand Down
18 changes: 14 additions & 4 deletions open-codegen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,20 @@ Note: This is the Changelog file of `opengen` - the Python interface of OpEn

### Added

- Implementation/testing of projection on simplices (`simplex.py`) (addresses #234)
- Implementation/testing of projection on Ball-1 (`ball1.py`) (addresses #238)
* Implementation/testing of projection on simplices (`simplex.py`) (addresses #234)
*-* Implementation/testing of projection on Ball-1 (`ball1.py`) (addresses #238)
* `# Safety` section in auto-generated unsafe auto-generated Rust code

### Removed

## [0.6.5] - 2020-10-21
* Unnecessary `#[no_mangle]`s in auto-generated Rust code

### Changed

* Took care of most clippy warnings in Rust and auto-generated Rust code


## [0.6.5] - 2021-04-16

### Changed

Expand All @@ -32,6 +41,7 @@ Note: This is the Changelog file of `opengen` - the Python interface of OpEn
### Fixed

* List of authors in `Cargo.toml` is generated properly
* Fixed bug when curvature is zero

### Changed

Expand Down Expand Up @@ -105,7 +115,7 @@ Note: This is the Changelog file of `opengen` - the Python interface of OpEn
* Project-specific `tcp_iface` TCP interface
* Fixed `lbfgs` typo

[0.6.6a1]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.5...opengen-0.6.6a1
[Unreleased]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.5...master
[0.6.5]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.4...opengen-0.6.5
[0.6.4]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.3...opengen-0.6.4
[0.6.3]: https://github.com/alphaville/optimization-engine/compare/opengen-0.6.2...opengen-0.6.3
Expand Down
1 change: 0 additions & 1 deletion open-codegen/opengen/icasadi/build.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use cc;
use std::path::Path;

fn main() {
Expand Down
18 changes: 12 additions & 6 deletions open-codegen/opengen/templates/c/optimizer_cinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/// Solver cache (structure `{{meta.optimizer_name}}Cache`)
///
#[allow(non_camel_case_types)]
#[no_mangle]
pub struct {{meta.optimizer_name}}Cache {
cache: AlmCache,
}
Expand All @@ -19,7 +18,6 @@ impl {{meta.optimizer_name}}Cache {
/// Structure: `{{meta.optimizer_name}}ExitStatus`
#[allow(non_camel_case_types)]
#[repr(C)]
#[no_mangle]
pub enum {{meta.optimizer_name}}ExitStatus {
/// The algorithm has converged
///
Expand All @@ -40,7 +38,6 @@ pub enum {{meta.optimizer_name}}ExitStatus {
/// Structure: `{{meta.optimizer_name}}SolverStatus`
///
#[repr(C)]
#[no_mangle]
pub struct {{meta.optimizer_name}}SolverStatus {
/// Exit status
exit_status: {{meta.optimizer_name}}ExitStatus,
Expand Down Expand Up @@ -80,7 +77,7 @@ pub extern "C" fn {{meta.optimizer_name|lower}}_new() -> *mut {{meta.optimizer_n
/// Solve the parametric optimization problem for a given parameter
/// .
/// .
/// Arguments:
/// # Arguments:
/// - `instance`: re-useable instance of AlmCache, which should be created using
/// `{{meta.optimizer_name|lower}}_new` (and should be destroyed once it is not
/// needed using `{{meta.optimizer_name|lower}}_free`
Expand All @@ -94,10 +91,15 @@ pub extern "C" fn {{meta.optimizer_name|lower}}_new() -> *mut {{meta.optimizer_n
/// penalty parameter
/// .
/// .
/// Returns:
/// # Returns:
/// Instance of `{{meta.optimizer_name}}SolverStatus`, with the solver status
/// (e.g., number of inner/outer iterations, measures of accuracy, solver time,
/// and the array of Lagrange multipliers at the solution).
/// .
/// .
/// .
/// # Safety
/// All arguments must have been properly initialised
#[no_mangle]
pub unsafe extern "C" fn {{meta.optimizer_name|lower}}_solve(
instance: *mut {{meta.optimizer_name}}Cache,
Expand Down Expand Up @@ -160,7 +162,7 @@ pub unsafe extern "C" fn {{meta.optimizer_name|lower}}_solve(
Some({% if problem.dim_constraints_aug_lagrangian() == 0 %}_{% endif %}y) => {
{%- if problem.dim_constraints_aug_lagrangian() > 0 %}
let mut y_array : [f64; {{meta.optimizer_name|upper}}_N1] = [0.0; {{meta.optimizer_name|upper}}_N1];
y_array.copy_from_slice(&y);
y_array.copy_from_slice(y);
y_array
{% else %}
std::ptr::null::<c_double>()
Expand Down Expand Up @@ -197,6 +199,10 @@ pub unsafe extern "C" fn {{meta.optimizer_name|lower}}_solve(

/// Deallocate the solver's memory, which has been previously allocated
/// using `{{meta.optimizer_name|lower}}_new`
///
///
/// # Safety
/// All arguments must have been properly initialised
#[no_mangle]
pub unsafe extern "C" fn {{meta.optimizer_name|lower}}_free(instance: *mut {{meta.optimizer_name}}Cache) {
// Add impl
Expand Down
12 changes: 6 additions & 6 deletions open-codegen/opengen/templates/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ fn make_constraints() -> impl Constraint {
// - Halfspace:
let offset: f64 = {{problem.constraints.offset}};
let normal_vector: &[f64] = &[{{problem.constraints.normal_vector | join(', ')}}];
Halfspace::new(&normal_vector, offset)
Halfspace::new(normal_vector, offset)
{% elif 'NoConstraints' == problem.constraints.__class__.__name__ -%}
// - No constraints (whole Rn):
NoConstraints::new()
Expand Down Expand Up @@ -302,20 +302,20 @@ pub fn solve(
assert_eq!(u.len(), {{meta.optimizer_name|upper}}_NUM_DECISION_VARIABLES, "Wrong number of decision variables (u)");

let psi = |u: &[f64], xi: &[f64], cost: &mut f64| -> Result<(), SolverError> {
icasadi_{{meta.optimizer_name}}::cost(&u, &xi, &p, cost);
icasadi_{{meta.optimizer_name}}::cost(u, xi, p, cost);
Ok(())
};
let grad_psi = |u: &[f64], xi: &[f64], grad: &mut [f64]| -> Result<(), SolverError> {
icasadi_{{meta.optimizer_name}}::grad(&u, &xi, &p, grad);
icasadi_{{meta.optimizer_name}}::grad(u, xi, p, grad);
Ok(())
};
{% if problem.dim_constraints_aug_lagrangian() > 0 %}
let f1 = |u: &[f64], res: &mut [f64]| -> Result<(), SolverError> {
icasadi_{{meta.optimizer_name}}::mapping_f1(&u, &p, res);
icasadi_{{meta.optimizer_name}}::mapping_f1(u, p, res);
Ok(())
};{% endif %}
{% if problem.dim_constraints_penalty() %}let f2 = |u: &[f64], res: &mut [f64]| -> Result<(), SolverError> {
icasadi_{{meta.optimizer_name}}::mapping_f2(&u, &p, res);
icasadi_{{meta.optimizer_name}}::mapping_f2(u, p, res);
Ok(())
};{% endif -%}
let bounds = make_constraints();
Expand Down Expand Up @@ -353,7 +353,7 @@ pub fn solve(
// initial vector of Lagrange multipliers, if provided;
// returns the problem status (instance of `AlmOptimizerStatus`)
if let Some(y0_) = y0 {
let mut alm_optimizer = alm_optimizer.with_initial_lagrange_multipliers(&y0_);
let mut alm_optimizer = alm_optimizer.with_initial_lagrange_multipliers(y0_);
alm_optimizer.solve(u)
} else {
alm_optimizer.solve(u)
Expand Down
6 changes: 3 additions & 3 deletions open-codegen/opengen/templates/tcp/tcp_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ fn execution_handler(
write_error_message(stream, 1600, "Initial guess has incompatible dimensions");
return;
}
u.copy_from_slice(&u0);
u.copy_from_slice(u0);
}
}

Expand All @@ -176,8 +176,8 @@ fn execution_handler(
write_error_message(stream, 3003, "wrong number of parameters");
return;
}
p.copy_from_slice(&parameter);
let status = solve(&p,
p.copy_from_slice(parameter);
let status = solve(p,
cache,
u,
&execution_parameter.initial_lagrange_multipliers,
Expand Down
14 changes: 7 additions & 7 deletions src/alm/alm_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ where
if let Some(f2) = &self.mapping_f2 {
let c = xi[0];
let mut z = vec![0.0; self.n2];
f2(&u, &mut z)?;
f2(u, &mut z)?;
*cost += 0.5 * c * matrix_operations::norm2_squared(&z);
}
Ok(())
Expand Down Expand Up @@ -281,8 +281,8 @@ where
let mut s_aux_var = vec![0.0; ny]; // auxiliary variable `s`
let y_lagrange_mult = &xi[1..];
let mut jac_prod = vec![0.0; nu];
mapping_f1(&u, &mut f1_u_plus_y_over_c)?; // f1_u_plus_y_over_c = F1(u)
// f1_u_plus_y_over_c = F1(u) + y/c
mapping_f1(u, &mut f1_u_plus_y_over_c)?; // f1_u_plus_y_over_c = F1(u)
// f1_u_plus_y_over_c = F1(u) + y/c
f1_u_plus_y_over_c
.iter_mut()
.zip(y_lagrange_mult.iter())
Expand All @@ -296,7 +296,7 @@ where
.zip(s_aux_var.iter())
.for_each(|(ti, si)| *ti -= si);

jf1t(&u, &f1_u_plus_y_over_c, &mut jac_prod)?;
jf1t(u, &f1_u_plus_y_over_c, &mut jac_prod)?;

// grad += c*t
grad.iter_mut()
Expand All @@ -309,9 +309,9 @@ where
let c = xi[0];
let mut f2u_aux = vec![0.0; self.n2];
let mut jf2u_times_f2u_aux = vec![0.0; self.n2];
f2(&u, &mut f2u_aux)?; // f2u_aux = F2(u)
jf2(&u, &f2u_aux, &mut jf2u_times_f2u_aux)?; // jf2u_times_f2u_aux = JF2(u)'*f2u_aux
// = JF2(u)'*F2(u)
f2(u, &mut f2u_aux)?; // f2u_aux = F2(u)
jf2(u, &f2u_aux, &mut jf2u_times_f2u_aux)?; // jf2u_times_f2u_aux = JF2(u)'*f2u_aux
// = JF2(u)'*F2(u)

// grad += c * jf2u_times_f2u_aux
grad.iter_mut()
Expand Down
13 changes: 6 additions & 7 deletions src/alm/alm_optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ where
let alm_cache = &mut self.alm_cache; // ALM cache
if let (Some(y_plus), Some(xi)) = (&alm_cache.y_plus, &alm_cache.xi) {
// compute ||y_plus - y||
let norm_diff_squared = matrix_operations::norm2_squared_diff(&y_plus, &xi[1..]);
let norm_diff_squared = matrix_operations::norm2_squared_diff(y_plus, &xi[1..]);
alm_cache.delta_y_norm_plus = norm_diff_squared.sqrt();
}
Ok(())
Expand Down Expand Up @@ -714,11 +714,11 @@ where
// psi(u) = psi(u; xi) and psi_grad(u) = phi_grad(u; xi)
// psi: R^nu --> R
let psi = |u: &[f64], psi_val: &mut f64| -> FunctionCallResult {
(alm_problem.parametric_cost)(u, &xi, psi_val)
(alm_problem.parametric_cost)(u, xi, psi_val)
};
// psi_grad: R^nu --> R^nu
let psi_grad = |u: &[f64], psi_grad: &mut [f64]| -> FunctionCallResult {
(alm_problem.parametric_gradient)(u, &xi, psi_grad)
(alm_problem.parametric_gradient)(u, xi, psi_grad)
};
// define the inner problem
let inner_problem = Problem::new(&self.alm_problem.constraints, psi_grad, psi);
Expand All @@ -732,7 +732,7 @@ where
.with_max_duration(
alm_cache
.available_time
.unwrap_or(std::time::Duration::from_secs(std::u64::MAX)),
.unwrap_or_else(|| std::time::Duration::from_secs(std::u64::MAX)),
)
// Set the maximum number of inner iterations
.with_max_iter(self.max_inner_iterations);
Expand Down Expand Up @@ -817,7 +817,7 @@ where
cache.delta_y_norm = cache.delta_y_norm_plus;
cache.f2_norm = cache.f2_norm_plus;
if let (Some(xi), Some(y_plus)) = (&mut cache.xi, &cache.y_plus) {
xi[1..].copy_from_slice(&y_plus);
xi[1..].copy_from_slice(y_plus);
}
cache.panoc_cache.reset();
}
Expand Down Expand Up @@ -979,8 +979,7 @@ where
.with_cost(cost);
if self.alm_problem.n1 > 0 {
let status = status.with_lagrange_multipliers(
&self
.alm_cache
self.alm_cache
.y_plus
.as_ref()
.expect("Although n1 > 0, there is no vector y (Lagrange multipliers)"),
Expand Down
2 changes: 1 addition & 1 deletion src/alm/alm_optimizer_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl AlmOptimizerStatus {
pub(crate) fn with_lagrange_multipliers(mut self, lagrange_multipliers: &[f64]) -> Self {
self.lagrange_multipliers = Some(vec![]);
if let Some(y) = &mut self.lagrange_multipliers {
y.extend_from_slice(&lagrange_multipliers);
y.extend_from_slice(lagrange_multipliers);
}
self
}
Expand Down
2 changes: 1 addition & 1 deletion src/constraints/finite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl<'a> Constraint for FiniteSet<'a> {
best_distance = dist;
}
}
x.copy_from_slice(&self.data[idx]);
x.copy_from_slice(self.data[idx]);
}

fn is_convex(&self) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion src/core/fbs/fbs_optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ where
let mut cost_value: f64 = 0.0;
(self.fbs_engine.problem.cost)(u, &mut cost_value)?;

if !matrix_operations::is_finite(&u) || !cost_value.is_finite() {
if !matrix_operations::is_finite(u) || !cost_value.is_finite() {
return Err(SolverError::NotFiniteComputation);
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/panoc/panoc_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ where
let gamma = self.cache.gamma;

// u_plus ← u - (1-tau)*gamma_fpr + tau*direction
self.compute_u_plus(&u);
self.compute_u_plus(u);

// Note: Here `cache.cost_value` and `cache.gradient_u` are overwritten
// with the values of the cost and its gradient at the next (candidate)
Expand Down
2 changes: 1 addition & 1 deletion src/core/panoc/panoc_optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ where
}

// check for possible NaN/inf
if !matrix_operations::is_finite(&u) {
if !matrix_operations::is_finite(u) {
return Err(SolverError::NotFiniteComputation);
}

Expand Down
2 changes: 1 addition & 1 deletion src/lipschitz_estimator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ where
/// If `estimate_local_lipschitz` has not been computed, the result
/// will point to a zero vector.
pub fn get_function_value(&self) -> &[f64] {
&self.function_value_at_u
self.function_value_at_u
}

///
Expand Down

0 comments on commit 6580b17

Please sign in to comment.