Skip to content

Commit fd0d2ad

Browse files
authored
Minor Cleanups (#14)
* refactor model build error * refactor model error and levmar error * remove snafu dependency * minor changes to documentation * enforce docs * use uninitialized matrix for temp storage instead instead of zero initialized matrix (in diag matrix multiplication) * replace zero initialized matrices with uninitialized where feasible * bump patch version number * fmt * update changelog * change codecov warnings to get around bogus fails * yaml fixed? * try and fix yaml again * change coverage backend to coveralls and overhaul respective github workflow * try fix coverage * try fix coverage * try fix coverage * try fix coverage * fixed coverage and added coveralls badge * update changelog
1 parent 782dc49 commit fd0d2ad

File tree

16 files changed

+163
-101
lines changed

16 files changed

+163
-101
lines changed

.github/workflows/coverage.yml

+11-29
Original file line numberDiff line numberDiff line change
@@ -7,36 +7,18 @@ on:
77
branches: [ main ]
88

99
env:
10-
CARGO_TERM_COLOR: always
11-
10+
RUST_BACKTRACE: 1
1211

1312
jobs:
14-
check:
15-
name: Rust project
16-
runs-on: ubuntu-latest
13+
test:
14+
name: coverage
15+
runs-on: ubuntu-latest
16+
container:
17+
image: xd009642/tarpaulin:develop-nightly
18+
options: --security-opt seccomp=unconfined
1719
steps:
18-
- name: Checkout repository
19-
uses: actions/checkout@v2
20-
21-
- name: Install stable toolchain
22-
uses: actions-rs/toolchain@v1
23-
with:
24-
toolchain: stable
25-
override: true
26-
27-
- name: Run cargo-tarpaulin
28-
uses: actions-rs/tarpaulin@v0.1
29-
with:
30-
#version: '0.15.0'
31-
args: '-- --test-threads 1'
32-
33-
- name: Upload to codecov.io
34-
uses: codecov/codecov-action@v3
35-
with:
36-
token: ${{secrets.CODECOV_TOKEN}}
20+
- name: Checkout repository
21+
uses: actions/checkout@v2
3722

38-
- name: Archive code coverage results
39-
uses: actions/upload-artifact@v1
40-
with:
41-
name: code-coverage-report
42-
path: cobertura.xml
23+
- name: Generate code coverage
24+
run: cargo +nightly tarpaulin --verbose --all-features --workspace --timeout 120 --coveralls ${{ secrets.COVERALLS_TOKEN }}

CHANGES.md

+12-2
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,20 @@
22

33
This is the changelog for the `varpro` library. See also here for a [version history](https://crates.io/crates/varpro/versions).
44

5+
## 0.4.1
6+
- Remove snafu dependency and replace by `thiserror`
7+
- Use uninitialized matrices instead of zero initialisation for places where contents will be overwritten anyways
8+
- Fix new clippy lints
9+
- Redo the code coverage workflow and switch to coveralls from codecov
10+
11+
## 0.3.0, 0.4.0
12+
Upgrade dependencies
13+
14+
515
## 0.2.0
616

7-
* Update `levenberg_marquardt` dependency to 0.8.0
8-
* Update `nalgebra` dependency to 0.25.3
17+
- Update `levenberg_marquardt` dependency to 0.8.0
18+
- Update `nalgebra` dependency to 0.25.3
919

1020
## 0.1.0
1121
Initial release

Cargo.toml

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "varpro"
3-
version = "0.4.0"
3+
version = "0.4.1"
44
authors = ["geo-ant"]
55
edition = "2021"
66
license = "MIT"
@@ -14,7 +14,7 @@ keywords = ["nonlinear","least","squares","fitting","regression"]
1414
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
1515

1616
[dependencies]
17-
snafu = "0.7"
17+
thiserror = "1"
1818
levenberg-marquardt = "0.12"
1919
nalgebra = "0.30"
2020
num-traits = "0.2"

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
![build](https://github.com/geo-ant/varpro/workflows/build/badge.svg?branch=main)
44
![tests](https://github.com/geo-ant/varpro/workflows/tests/badge.svg?branch=main)
55
![lints](https://github.com/geo-ant/varpro/workflows/lints/badge.svg?branch=main)
6-
[![coverage](https://codecov.io/gh/geo-ant/varpro/branch/main/graph/badge.svg?token=1L2PEJFMXP)](https://codecov.io/gh/geo-ant/varpro)
6+
[![coverage](https://coveralls.io/repos/github/geo-ant/varpro/badge.svg?branch=)](https://coveralls.io/github/geo-ant/varpro?branch=)
77

88
This library enables robust and fast least-squares fitting of nonlinear, separable model functions to observations. It uses the VarPro algorithm to achieve this.
99

codecov.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ coverage:
44
status:
55
project:
66
default:
7-
threshold: 5% # allow for small deviation from base
7+
threshold: 11% # allow for small deviation from base
88
patch:
99
default:
10-
threshold: 5% # allow for small deviation from base
10+
threshold: 11% # allow for small deviation from base
1111

1212
ignore: # ignore codecoverage on following paths
1313
- ".github"

src/lib.rs

+26-10
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,42 @@
1+
#![deny(missing_docs)]
12
//! The varpro crate enables nonlinear least squares fitting for separable models using the Variable Projection (VarPro) algorithm.
23
//!
34
//! # Introduction
4-
//! A large class of nonlinear models consists of a mixture of truly nonlinear as well as linear model
5-
//! parameters. These are called *separable models* which can be written as a linear combination
6-
//! of `$N_{basis}$` nonlinear model basis functions. The purpose of this crate provide a simple interface to
7-
//! robust and fast routines to fit separable models to data. Consider a data vector `$\vec{y}= (y_1,\dots,y_{N_{data}})^T$`, which
5+
//!
6+
//! A large class of nonlinear models consists of a mixture of both truly nonlinear and _linear_ model
7+
//! parameters. These are called *separable models* and can be written as a linear combination
8+
//! of `$N_{basis}$` nonlinear model basis functions. The purpose of this crate is to fit linear
9+
//! models to data using fast and robust algorithms, while providing a simple interface.
10+
//!
11+
//! Consider a data vector `$\vec{y}= (y_1,\dots,y_{N_{data}})^T$`, which
812
//! is sampled at grid points `$\vec{x}=(x_1,\dots,x_{N_{data}})^T$`, both with `$N_{data}$` elements. Our goal is to fit a nonlinear,
9-
//! separable model to the data. Because the model is separable we can write it as
13+
//! separable model `$f$` to the data. Because the model is separable we can write it as
1014
//!
1115
//! ```math
1216
//! \vec{f}(\vec{x},\vec{\alpha},\vec{c}) = \sum_{j=1}^{N_{basis}} c_j \vec{f}_j(\vec{x},S_j(\alpha))
1317
//! ```
18+
//!
1419
//! Lets look at the components of this equation in more detail. The vector valued function
1520
//! `$\vec{f}(\vec{x},\vec{\alpha},\vec{c})$` is the actual model we want to fit. It depends on
1621
//! three arguments:
17-
//! * `$\vec{x}$` is the independent variable which corresponds to the grid points. It can be a time,
18-
//! location or anything else.
22+
//! * `$\vec{x}$` is the independent variable which corresponds to the grid points. Those can be a time,
23+
//! location or anything at all, really.
1924
//! * `$\vec{\alpha}=(\alpha_1,\dots,\alpha_{N_{params}})^T$` is the vector of nonlinear model parameters.
2025
//! We will get back to these later.
2126
//! * `$\vec{c}=(c_1,\dots,c_{N_{basis}})^T$` is the vector of coefficients for the basis functions.
2227
//!
28+
//! Note that we call `$\vec{\alpha}$` the _parameters_ and `$\vec{c}$` the _coefficients_ of the model.
29+
//! We do this do make the distincion between _nonlinear parameters_ and _linear coefficients_.
30+
//! Of course the model itself is parametrized on both `$\vec{\alpha}$` and `$\vec{c}$`.
31+
//!
2332
//! ## Model Parameters and Basis Functions
2433
//!
2534
//! The model itself is given as a linear combination of *nonlinear* basis functions `$\vec{f}_j$` with
2635
//! expansion coefficient `$c_j$`. The basis functions themselves only depend on the independent variable
2736
//! `$\vec{x}$` and on a subset `$S_j(\alpha)$` of the nonlinear model parameters `$\vec{\alpha}$`.
28-
//! Each basis function can depend on a different subset.
37+
//! Each basis function can depend on a different subset, but there is no restriction on which
38+
//! parameters a function can depend. Arbitrary functions might share some parameters. It's also
39+
//! fine for functions to depend on some (or only) parameters that are exclusive to them.
2940
//!
3041
//! ## What VarPro Computes
3142
//! This crate finds the parameters `$\vec{\alpha}$` and `$\vec{c}$` that
@@ -61,7 +72,7 @@
6172
//! 3. Cast the fitting problem into a [LevMarProblem](crate::solvers::levmar::LevMarProblem) using
6273
//! the [LevMarProblemBuilder](crate::solvers::levmar::builder::LevMarProblemBuilder).
6374
//! 4. Solve the fitting problem using the [LevMarSolver](crate::solvers::levmar::LevMarSolver), which
64-
//! is an alias for the [LevenbergMarquardt](levenberg_marquardt::LevenbergMarquardt) and allows to set
75+
//! is an alias for the [LevenbergMarquardt](levenberg_marquardt::LevenbergMarquardt) struct and allows to set
6576
//! additional parameters of the algorithm before performing the minimization.
6677
//! 5. Check the minimization report and, if successful, retrieve the nonlinear parameters `$\alpha$`
6778
//! using the [LevMarProblem::params](levenberg_marquardt::LeastSquaresProblem::params) and the linear
@@ -75,7 +86,7 @@
7586
//! function with the same number of elements as `$\vec{x}$` and `$\vec{y}$`. The component at index
7687
//! `k` is given by
7788
//! ```math
78-
//! f_k(\vec{x},\vec{\alpha},\vec{c})= c_1 \exp\left(-x_k/\tau_1\right)+c_2 \exp\left(-x_k/\tau_2\right)+c_3,
89+
//! (\vec{f}(\vec{x},\vec{\alpha},\vec{c}))_k= c_1 \exp\left(-x_k/\tau_1\right)+c_2 \exp\left(-x_k/\tau_2\right)+c_3,
7990
//! ```
8091
//! which is just a fancy way of saying that the exponential functions are applied element-wise to the vector `$\vec{x}$`.
8192
//!
@@ -168,9 +179,14 @@
168179
//! **attention**: the O'Leary paper contains errors that are fixed in [this blog article](https://geo-ant.github.io/blog/2020/variable-projection-part-1-fundamentals/) of mine.
169180
//!
170181
//! (Golub2003) Golub, G. , Pereyra, V Separable nonlinear least squares: the variable projection method and its applications. Inverse Problems **19** R1 (2003) [https://iopscience.iop.org/article/10.1088/0266-5611/19/2/201](https://iopscience.iop.org/article/10.1088/0266-5611/19/2/201)
182+
183+
/// helper implementation to make working with basis functions more seamless
171184
pub mod basis_function;
185+
/// code pertaining to building and working with separable models
172186
pub mod model;
187+
/// commonly useful imports
173188
pub mod prelude;
189+
/// solvers for the nonlinear minimization problem
174190
pub mod solvers;
175191

176192
/// private module that contains helper functionality for linear algebra that is not yet

src/linalg_helpers/mod.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#[cfg(test)]
22
mod test;
33

4-
use nalgebra::{ClosedMul, ComplexField, DMatrix, DVector, Scalar};
4+
use nalgebra::{ClosedMul, ComplexField, DMatrix, DVector, Dynamic, Scalar};
55
use std::ops::Mul;
66

77
/// A square diagonal matrix with dynamic dimension. Off-diagonal entries are assumed zero.
@@ -70,11 +70,18 @@ where
7070
"Matrix dimensions incorrect for diagonal matrix multiplication."
7171
);
7272

73-
let mut result_matrix = DMatrix::<ScalarType>::zeros(self.nrows(), rhs.ncols());
73+
// this isn't an awesome pattern, but it is safe since we fill the Matrix
74+
// with valid values below. Ideally we would first call the
75+
// copy_from functionality below, but we can't because the Scalar
76+
// trait bounds will screw us. See https://github.com/dimforge/nalgebra/pull/949
77+
let mut result_matrix = unsafe {
78+
DMatrix::uninit(Dynamic::new(self.nrows()), Dynamic::new(rhs.ncols())).assume_init()
79+
};
7480

7581
for (col_idx, mut col) in result_matrix.column_iter_mut().enumerate() {
7682
col.copy_from(&self.diagonal.component_mul(&rhs.column(col_idx)));
7783
}
84+
7885
result_matrix
7986
}
8087
}

src/linalg_helpers/test.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ fn diagonal_matrix_matrix_and_matrix_vector_product_produces_correct_results() {
1313

1414
let v = DVector::from(vec![5., 8., 6., 2., 5.]);
1515

16-
let mut A = DMatrix::from_element(ndiag, 2, 0.);
16+
let mut A = DMatrix::zeros(ndiag, 2);
1717
A.set_column(0, &DVector::from(vec![2., 6., 4., 9., 5.]));
1818
A.set_column(1, &DVector::from(vec![3., 9., 2., 1., 0.]));
1919

src/model/builder/error.rs

+45-24
Original file line numberDiff line numberDiff line change
@@ -1,85 +1,106 @@
1-
use snafu::Snafu;
1+
use thiserror::Error as ThisError;
22

33
/// An error structure that contains error variants that occur when building a model.
4-
#[derive(Debug, Clone, Snafu, PartialEq, Eq)]
5-
#[snafu(visibility(pub))]
4+
#[derive(Debug, Clone, PartialEq, Eq, ThisError)]
65
pub enum ModelBuildError {
76
/// Model or function parameters contain duplicates
8-
#[snafu(display("Parameter list {:?} contains duplicates! Parameter lists must comprise only unique elements.",function_parameters))]
9-
DuplicateParameterNames { function_parameters: Vec<String> },
7+
#[error("Parameter list {:?} contains duplicates! Parameter lists must comprise only unique elements.",function_parameters)]
8+
DuplicateParameterNames {
9+
/// the given parameter list containing duplicates
10+
function_parameters: Vec<String>,
11+
},
1012

1113
/// Model or function parameter list is empty. To add functions that are independent of
1214
/// model parameters, use the interface for adding invariant functions.
13-
#[snafu(display(
15+
#[error(
1416
"A function or model parameter list is empty! It must at least contain one parameter."
15-
))]
17+
)]
1618
EmptyParameters,
1719

1820
/// A function was added to the model which depends on parameters which are not in the model
19-
#[snafu(display(
21+
#[error(
2022
"Function parameter '{}' is not part of the model parameters.",
2123
function_parameter
22-
))]
23-
FunctionParameterNotInModel { function_parameter: String },
24+
)]
25+
FunctionParameterNotInModel {
26+
/// the name of the parameter not in the set
27+
function_parameter: String,
28+
},
2429

2530
/// Tried to provide a partial derivative with respect to a parameter that a function does
2631
/// not depend on
27-
#[snafu(display(
32+
#[error(
2833
"Parameter '{}' given for partial derivative does not exist in parameter list '{:?}'.",
2934
parameter,
3035
function_parameters
31-
))]
36+
)]
3237
InvalidDerivative {
38+
/// paramter where a derivative was provided
3339
parameter: String,
40+
/// the actual parameters this function depends on
3441
function_parameters: Vec<String>,
3542
},
3643

3744
/// Tried to provide the same partial derivative twice.
38-
#[snafu(display("Derivative for parameter '{}' was already provided! Give each partial derivative exactly once.", parameter))]
39-
DuplicateDerivative { parameter: String },
45+
#[error("Derivative for parameter '{}' was already provided! Give each partial derivative exactly once.", parameter)]
46+
DuplicateDerivative {
47+
/// the name of the derivative specified twice
48+
parameter: String,
49+
},
4050

4151
/// Not all partial derivatives for a function where given. Each function must be given
4252
/// a partial derivative with respect to each parameter it depends on.
43-
#[snafu(display(
53+
#[error(
4454
"Function with paramter list {:?} is missing derivative for parametr '{}'.",
4555
function_parameters,
4656
missing_parameter
47-
))]
57+
)]
4858
MissingDerivative {
59+
/// this parameter misses a derivative
4960
missing_parameter: String,
61+
/// the parameters that this function depends on
5062
function_parameters: Vec<String>,
5163
},
5264

5365
/// Tried to construct a model without base functions
54-
#[snafu(display(
66+
#[error(
5567
"Tried to construct model with no functions. A model must contain at least one function."
56-
))]
68+
)]
5769
EmptyModel,
5870

5971
/// The model depends on a certain parameter that none of the base functions depend on.
60-
#[snafu(display("Model depends on parameter '{}', but none of its functions use it. Each model parameter must occur in at least one function.",parameter))]
61-
UnusedParameter { parameter: String },
72+
#[error("Model depends on parameter '{}', but none of its functions use it. Each model parameter must occur in at least one function.",parameter)]
73+
UnusedParameter {
74+
/// the unused parameter name
75+
parameter: String,
76+
},
6277

6378
/// This error indicates that the more or fewer string parameters where provided as function
6479
/// parameters than the actual variadic function takes. This might accidentally happen when giving
6580
/// a derivative that does not depend on a certain parameter, whereas the base function does.
6681
/// However, the library requires that the derivatives take all the parameters its base function
6782
/// takes in the same order.
68-
#[snafu(display(
83+
#[error(
6984
"Incorrect parameter count: Given function parameters '{:?}' have length {}, but the provided function takes {} parameter arguments.",
7085
params,
7186
string_params_count,
7287
function_argument_count,
73-
))]
88+
)]
7489
IncorrectParameterCount {
90+
/// the parameters that the function actually depends on
7591
params: Vec<String>,
92+
/// the number of parameters provided through the string api
7693
string_params_count: usize,
94+
/// the number of arguments this function actually takes
7795
function_argument_count: usize,
7896
},
7997

8098
/// Parameter names may not contain a comma separator, because this is most likely caused by a typo, i.e.
8199
/// `["tau,phi"]`, instead of actually `["tau","phi"]`. So this is forbidden in order to help spotting these
82100
/// hard to find errors.
83-
#[snafu(display("Parameter names may not contain comma separator: '{}'. Did you want to give two parameters?",param_name))]
84-
CommaInParameterNameNotAllowed { param_name: String },
101+
#[error("Parameter names may not contain comma separator: '{}'. Did you want to give two parameters?",param_name)]
102+
CommaInParameterNameNotAllowed {
103+
/// the parameter name
104+
param_name: String,
105+
},
85106
}

src/model/builder/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod modelfunction_builder;
1212
#[cfg(test)]
1313
mod test;
1414

15+
/// contains the error for the model builder
1516
pub mod error;
1617

1718
///! A builder that allows us to construct a valid [SeparableModel](crate::model::SeparableModel).

0 commit comments

Comments
 (0)