docs: clarify determinism guarantees

README.md

@@ -41,6 +41,15 @@ For a more complex example, see the [resource allocation problem](https://github
   There is no solving algorithm in good_lp itself. If you have an issue with a solver,
   report it to the solver directly. See below for the list of supported solvers.
 
+## Determinism
+
+good_lp deterministically constructs a model when variables, coefficients, and constraints are
+supplied in a deterministic order. In particular, an unordered collection such as a `HashMap`
+can make the order of dynamically added variables or constraints non-deterministic.
+
+To the best of our knowledge, none of the supported solvers provides a determinism guarantee.
+Consequently, good_lp makes no guarantee that solving a model returns deterministic results.
+
 ### Contributing
 
 Pull requests are welcome !

src/lib.rs

@@ -32,6 +32,18 @@
 //! This crate supports multiple solvers,
 //! that can be activated using [feature flags](https://docs.rs/crate/good_lp/latest/features).
 //!
+//! ## Determinism
+//!
+//! good_lp guarantees deterministic model construction when variables, coefficients, and
+//! constraints are supplied in a deterministic order. In particular, the iterator order passed to
+//! [ProblemVariables::add_all] and [SolverModel::with_all] determines the order in the
+//! generated model. Do not pass an unordered collection such as a
+//! [`std::collections::HashMap`] directly to these APIs when model order matters.
+//!
+//! A call to [SolverModel::solve] returns deterministic results only when the underlying solver
+//! guarantees deterministic results. good_lp does not make this guarantee for any solver; see
+//! the documentation for its solver factory.
+//!
 //! ## Usage
 //!
 //! You initially create your variables using [variables] and [ProblemVariables::add].

src/solvers/mod.rs

@@ -199,7 +199,10 @@ pub trait SolverModel {
         self
     }
 
-    /// Takes a model and adds a list of constraints to it
+    /// Takes a model and adds a list of constraints to it, in iterator order.
+    ///
+    /// Constraint order can affect a solver's result for problems with multiple optimal
+    /// solutions. Pass an iterator with a deterministic order when reproducibility matters.
     ///
     /// # Examples
     /// ```rust
@@ -224,7 +227,10 @@ pub trait SolverModel {
         self
     }
 
-    /// Find the solution for the problem being modeled
+    /// Find the solution for the problem being modeled.
+    ///
+    /// This function returns deterministic results only when the underlying solver guarantees
+    /// deterministic results. good_lp does not make a general solver determinism guarantee.
     fn solve(self) -> Result<Self::Solution, Self::Error>;
 
     /// Adds a constraint to the Model and returns a reference to the index
@@ -513,3 +519,44 @@ pub trait WithMipGap {
     where
         Self: Sized;
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::variables;
+
+    #[derive(Default)]
+    struct RecordingModel {
+        constants: Vec<f64>,
+    }
+
+    impl SolverModel for RecordingModel {
+        type Solution = HashMap<Variable, f64>;
+        type Error = ResolutionError;
+
+        fn solve(self) -> Result<Self::Solution, Self::Error> {
+            unreachable!("this test only verifies model construction")
+        }
+
+        fn add_constraint(&mut self, constraint: Constraint) -> ConstraintReference {
+            let index = self.constants.len();
+            self.constants.push(constraint.expression.constant);
+            ConstraintReference { index }
+        }
+
+        fn name() -> &'static str {
+            "recording"
+        }
+    }
+
+    #[test]
+    fn with_all_adds_constraints_in_iterator_order() {
+        variables! {variables:
+            x;
+            y;
+        }
+        let model = RecordingModel::default().with_all([x << 1, y << 2]);
+
+        assert_eq!(model.constants, [-1., -2.]);
+    }
+}

src/variable.rs

@@ -412,7 +412,10 @@ impl ProblemVariables {
         Variable::at(index)
     }
 
-    /// Adds many variables with the given definitions
+    /// Adds many variables with the given definitions.
+    ///
+    /// Variables receive indices in iterator order. To construct a deterministic model, pass an
+    /// iterator with a deterministic order rather than an unordered collection.
     ///
     /// ```
     /// use good_lp::*;
@@ -522,7 +525,7 @@ impl ProblemVariables {
         self.optimise(ObjectiveDirection::Minimisation, objective)
     }
 
-    /// Iterates over the couples of variables with their properties
+    /// Iterates over the couples of variables with their properties, in variable-index order.
     pub fn iter_variables_with_def(&self) -> impl Iterator<Item = (Variable, &VariableDefinition)> {
         self.variables
             .iter()
@@ -690,3 +693,30 @@ impl Not for Variable {
         1. - self
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn add_all_assigns_variables_in_iterator_order() {
+        let mut variables = ProblemVariables::new();
+        let added: Vec<_> = variables.add_all([
+            variable().name("first"),
+            variable().name("second"),
+            variable().name("third"),
+        ]);
+
+        assert_eq!(
+            added.iter().map(Variable::index).collect::<Vec<_>>(),
+            [0, 1, 2]
+        );
+        assert_eq!(
+            variables
+                .iter_variables_with_def()
+                .map(|(_, definition)| definition.get_name())
+                .collect::<Vec<_>>(),
+            ["first", "second", "third"]
+        );
+    }
+}