Skip to content

Adding a bit more documentation + clarifying some interfaces#2

Merged
kwesiRutledge merged 20 commits intomainfrom
kr/feature/gonum-bug1
Oct 17, 2025
Merged

Adding a bit more documentation + clarifying some interfaces#2
kwesiRutledge merged 20 commits intomainfrom
kr/feature/gonum-bug1

Conversation

@kwesiRutledge
Copy link
Contributor

@kwesiRutledge kwesiRutledge commented Oct 4, 2025

Summary

  • Added more to the main README.md
  • Added a package simplex_solution, which implements a solution object that will be returned by all simplex solvers.
  • Starting to work on a potential solution to the problem that caused Gonum some trouble: [Gonum issue]

@kwesiRutledge kwesiRutledge requested a review from Copilot October 4, 2025 18:43
@kwesiRutledge kwesiRutledge self-assigned this Oct 4, 2025
@codecov
Copy link

codecov bot commented Oct 4, 2025

Codecov Report

❌ Patch coverage is 11.29032% with 110 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.79%. Comparing base (58b26b5) to head (05e6179).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
algorithms/tableau/state.go 10.00% 36 Missing ⚠️
utils/tableau.go 20.00% 20 Missing ⚠️
algorithms/stanford/stanford.go 0.00% 15 Missing ⚠️
solution/simplex_solution.go 0.00% 13 Missing ⚠️
algorithms/tableau/tableau_algo.go 0.00% 10 Missing ⚠️
algorithms/tableau/errors.go 0.00% 8 Missing ⚠️
...lgorithms/tableau/termination/termination_types.go 0.00% 5 Missing ⚠️
simplexSolver/solver.go 0.00% 2 Missing ⚠️
algorithms/tableau/selection/blands_rule.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #2      +/-   ##
==========================================
- Coverage   41.89%   40.79%   -1.11%     
==========================================
  Files          15       17       +2     
  Lines        1518     1564      +46     
==========================================
+ Hits          636      638       +2     
- Misses        830      874      +44     
  Partials       52       52              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the simplex library to align with a new version of the MatProInterface.go dependency (v0.6.0) and introduces a new dedicated SimplexSolution type to replace the generic problem.Solution type.

  • Introduces a new SimplexSolution struct with clearer field naming (VariableValues instead of Values)
  • Updates all algorithm interfaces and implementations to use the new solution type
  • Updates dependency versions in go.mod to latest versions

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
solution/simplex_solution.go Defines new SimplexSolution struct with VariableValues, Objective, and Status fields
simplexSolver/solver.go Updates Solve method to return SimplexSolution instead of problem.Solution
go.mod Updates MatProInterface.go to v0.6.0 and SymbolicMath.go to v0.2.5
examples/sergiy_butenko1/the_simplex_method_part_ii.go Updates field access from Values to VariableValues
examples/gonum_bug1/try_bug.go New example file demonstrating simplex solver usage
algorithms/tableau/termination/termination_types.go Updates imports and return type for solution status
algorithms/tableau/tableau_algo.go Updates Solve method signature and solution construction
algorithms/tableau/state.go Updates solution creation and field naming
algorithms/stanford/stanford.go Updates solution construction to use new SimplexSolution type
algorithms/algorithm_interface.go Updates interface to return SimplexSolution
README.md Adds installation instructions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 204 to 211
for ii, bv := range state.BasicVariables {
solution.Values[bv.ID] = xBasic.AtVec(ii)
solution.VariableValues[bv.ID] = xBasic.AtVec(ii)
}

// Set the values of the non-basic variables
for jj, nv := range state.GetNonBasicVariables() {
solution.Values[nv.ID] = state.NonBasicValues.AtVec(jj)
solution.VariableValues[nv.ID] = state.NonBasicValues.AtVec(jj)
}
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The VariableValues map is not initialized before use. This will cause a panic when trying to assign values to a nil map.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will likely delete this soon.

Comment on lines +346 to +351

// Assemble Solution Output
return problem.Solution{
Values: map[uint64]float64{},
Objective: -1.0,
Status: currentStatus,
return simplex_solution.SimplexSolution{
VariableValues: map[uint64]float64{},
Objective: -1.0,
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded objective value of -1.0 appears to be a placeholder. This should either be calculated properly or documented as a placeholder value.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value was removed from the solution object.

@kwesiRutledge kwesiRutledge merged commit 419a58e into main Oct 17, 2025
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants