Skip to content

Commit 2cafb9b

Browse files
committed
feat: replace COIN-OR Clp with HiGHS solver engine
- Replace ClpSimplex with Highs class in dFBAModel - Use HighsModel with CSC matrix format for LP problem initialization - Update optimize() to use highs.run() and HighsSolution for results - Set ObjSense::kMaximize for FBA biomass maximization - Update bound setters to use highs.changeColBounds() - Add solver options: feasibility tolerances (1e-7), iteration limit (10000) - Update all dFBA Makefiles to link against -lhighs with proper RPATH - Fix sbml_filename XML path to reside within <settings> block - Remove all COIN-OR/Clp references from source code
1 parent c67aa23 commit 2cafb9b

15 files changed

Lines changed: 502 additions & 624 deletions

File tree

Makefile

Lines changed: 101 additions & 328 deletions
Large diffs are not rendered by default.

addons/dFBA/src/dfba_Model.cpp

Lines changed: 118 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ dFBAModel::dFBAModel()
1616
{
1717
this->id = "none";
1818
this->is_initialized = false;
19-
this->handler = NULL;
2019
this->solution.status = "none";
2120
}
2221

@@ -27,8 +26,6 @@ dFBAModel::~dFBAModel() {
2726
for(dFBAMetabolite* met: this->metabolites)
2827
delete met;
2928

30-
if (this->handler != nullptr)
31-
delete handler;
3229
}
3330

3431
dFBAModel::dFBAModel(const dFBAModel& copy) {
@@ -53,17 +50,11 @@ dFBAModel::dFBAModel(const dFBAModel& copy) {
5350
// Copy solution (assuming it can be copied by value)
5451
this->solution = copy.solution;
5552

56-
// Handle ClpSimplex problem copying (depends on how `ClpSimplex` needs to be cloned)
53+
// Re-initialize HiGHS problem for the copy
5754
if (copy.is_initialized) {
5855
this->initProblem();
5956
}
6057

61-
// Copy message handler
62-
if (copy.handler != nullptr) {
63-
this->handler = new CoinMessageHandler(*copy.handler);
64-
} else {
65-
this->handler = nullptr;
66-
}
6758
}
6859

6960

@@ -84,10 +75,6 @@ dFBAModel& dFBAModel::operator=(const dFBAModel& other) {
8475
}
8576
reactions.clear();
8677

87-
if (handler != nullptr) {
88-
delete handler;
89-
}
90-
9178
// Copy primitive members
9279
this->id = other.id;
9380
this->is_initialized = other.is_initialized;
@@ -109,17 +96,9 @@ dFBAModel& dFBAModel::operator=(const dFBAModel& other) {
10996
// Copy solution (assuming it can be copied by value)
11097
this->solution = other.solution;
11198

112-
// Handle ClpSimplex problem copying
99+
// Re-initialize HiGHS problem
113100
if (other.is_initialized) {
114101
this->initProblem();
115-
this->is_initialized = true;
116-
}
117-
118-
// Copy message handler
119-
if (other.handler != nullptr) {
120-
this->handler = new CoinMessageHandler(*other.handler);
121-
} else {
122-
this->handler = nullptr;
123102
}
124103

125104
return *this;
@@ -136,17 +115,12 @@ void dFBAModel::clear(){
136115
this->reactionsIndexer.clear();
137116
this->metaboliteIndexer.clear();
138117
this->solution.clear();
139-
this->problem = ClpSimplex();
140-
this->handler = NULL;
118+
highs.~Highs();
119+
new (&highs) Highs();
141120

142121
return;
143122
}
144123

145-
const ClpSimplex* dFBAModel::getLpModel() const
146-
{
147-
return &this->problem;
148-
}
149-
150124
const int dFBAModel::getNumReactions()
151125
{
152126
return this->reactions.size();
@@ -232,7 +206,7 @@ void dFBAModel::setReactionUpperBound(std::string rId, double upperBound)
232206
{
233207
rxn->setUpperBound(upperBound);
234208
int colIdx = this->reactionsIndexer[rId];
235-
this->problem.setColumnUpper(colIdx, upperBound);
209+
this->highs.changeColBounds(colIdx, rxn->getLowerBound(), upperBound);
236210
}
237211
else{
238212
std::cerr << "Reaction with ID " << rId << " not found in the model." << std::endl;
@@ -257,7 +231,7 @@ void dFBAModel::setReactionLowerBound(std::string rId, double lowerBound)
257231
{
258232
rxn->setLowerBound(lowerBound);
259233
int colIdx = this->reactionsIndexer[rId];
260-
this->problem.setColumnLower(colIdx, lowerBound);
234+
this->highs.changeColBounds(colIdx, lowerBound , rxn->getUpperBound());
261235
}
262236
else{
263237
std::cerr << "Reaction with ID " << rId << " not found in the model." << std::endl;
@@ -476,75 +450,95 @@ void dFBAModel::readSBMLModel(const char* sbmlFileName)
476450
delete document;
477451
}
478452

479-
void dFBAModel::initProblem()
480-
{
453+
void dFBAModel::initProblem() {
481454
int n_rows = this->getNumMetabolites();
482455
int n_cols = this->getNumReactions();
483456

484-
this->handler = new CoinMessageHandler(nullptr);
485-
// std::cout << "Initilizing LP problem n=" << n_rows << std::endl;
486-
this->handler->setLogLevel(0);
487-
this->problem.passInMessageHandler(this->handler);
488-
489-
CoinPackedMatrix matrix;
490-
matrix.setDimensions(n_rows, 0);
491-
492-
double* row_lb = new double[n_rows]; //the row lower bounds
493-
double* row_ub = new double[n_rows]; //the row upper bounds
494-
double* col_lb = new double[n_cols]; //the column lower bounds
495-
double* col_ub = new double[n_cols]; //the column upper bounds
496-
double* objective = new double[n_cols]; //the objective coefficients
497-
498-
for(int i=0; i< n_rows; i++)
499-
{
500-
row_lb[i] = 0;
501-
row_ub[i] = 0;
502-
}
503-
for(dFBAReaction* rxn: this->reactions)
504-
{
505-
int col_idx = this->reactionsIndexer[rxn->getId()];
506-
//std::cout << "Adding reaction " << rxn->getId() << " with index: " << this->reactionsIndexer[rxn->getId()] << " to the LP problem" << std::endl;
507-
col_lb[col_idx] = rxn->getLowerBound();
508-
col_ub[col_idx] = rxn->getUpperBound();
509-
objective[col_idx] = rxn->getObjectiveCoefficient();
510-
511-
const std::map<std::string, double>& metabolites = rxn->getMetabolites();
512-
513-
CoinPackedVector col;
514-
for (auto it = metabolites.begin(); it != metabolites.end(); ++it) {
515-
const std::string& metId = it->first; // Get metabolite ID
516-
double stoich_coeff = it->second;
517-
518-
// Use the metabolite ID to get the index from metaboliteIndexer
519-
auto idx_it = this->metaboliteIndexer.find(metId);
520-
if (idx_it != this->metaboliteIndexer.end()) {
521-
int row_idx = idx_it->second;
522-
col.insert(row_idx, stoich_coeff);
523-
}
457+
// Silence HiGHS output
458+
highs.setOptionValue("output_flag", false);
459+
460+
std::cout << "Initializing LP problem with " << n_rows << " constraints and "
461+
<< n_cols << " variables." << std::endl;
462+
463+
// Prepare LP data vectors.
464+
std::vector<double> row_lower(n_rows, 0.0); // All row lower bounds set to 0.
465+
std::vector<double> row_upper(n_rows, 0.0); // All row upper bounds set to 0.
466+
std::vector<double> col_lower(n_cols);
467+
std::vector<double> col_upper(n_cols);
468+
std::vector<double> col_cost(n_cols);
469+
470+
// To process reactions in order, create a vector indexed by column index.
471+
std::vector<dFBAReaction*> reactions_by_index(n_cols, nullptr);
472+
for (dFBAReaction* rxn : this->reactions) {
473+
int col_idx = this->reactionsIndexer[rxn->getId()];
474+
reactions_by_index[col_idx] = rxn;
475+
}
476+
477+
// Prepare the packed column-wise matrix data.
478+
std::vector<int> a_start(n_cols + 1, 0); // a_start[0] must be zero.
479+
std::vector<int> a_index; // Row indices for nonzeros.
480+
std::vector<double> a_value; // Nonzero values.
481+
482+
// Loop over each reaction (column).
483+
for (int j = 0; j < n_cols; j++) {
484+
dFBAReaction* rxn = reactions_by_index[j];
485+
// Set column bounds and objective coefficients.
486+
col_lower[j] = rxn->getLowerBound();
487+
col_upper[j] = rxn->getUpperBound();
488+
col_cost[j] = rxn->getObjectiveCoefficient();
489+
490+
// Record starting index for column j.
491+
a_start[j] = a_index.size();
492+
// For each metabolite in the reaction, add the nonzero entry.
493+
const std::map<std::string, double>& mets = rxn->getMetabolites();
494+
for (auto it = mets.begin(); it != mets.end(); ++it) {
495+
auto idx_it = this->metaboliteIndexer.find(it->first);
496+
if (idx_it != this->metaboliteIndexer.end()) {
497+
int row_idx = idx_it->second;
498+
a_index.push_back(row_idx);
499+
a_value.push_back(it->second);
524500
}
525-
matrix.appendCol(col);
526-
}
527-
528-
this->problem.loadProblem(matrix, col_lb, col_ub, objective, row_lb, row_ub);
529-
this->problem.setOptimizationDirection(-1);
530-
531-
this->problem.setPerturbation(50); // 50 is a standard default for perturbation. It means that the perturbation is always applied. 100 is the default value (automatic)
532-
533-
delete[] col_lb;
534-
delete[] col_ub;
535-
delete[] row_lb;
536-
delete[] row_ub;
537-
delete[] objective;
538-
539-
this->problem.setPrimalTolerance(1e-7);
540-
this->problem.setDualTolerance(1e-7);
541-
this->problem.scaling(3);
542-
this->problem.setMaximumIterations(10000);
543-
this->problem.setLogLevel(0);
544-
this->problem.initialSolve();
501+
}
502+
}
503+
// Final entry: total number of nonzeros.
504+
a_start[n_cols] = a_index.size();
505+
506+
// Build the HighsModel.
507+
HighsModel model;
508+
model.lp_.num_col_ = n_cols;
509+
model.lp_.num_row_ = n_rows;
510+
model.lp_.sense_ = ObjSense::kMaximize;
511+
model.lp_.offset_ = 0.0;
512+
model.lp_.col_cost_ = col_cost;
513+
model.lp_.col_lower_ = col_lower;
514+
model.lp_.col_upper_ = col_upper;
515+
model.lp_.row_lower_ = row_lower;
516+
model.lp_.row_upper_ = row_upper;
517+
518+
// Set the constraint matrix data.
519+
model.lp_.a_matrix_.format_ = MatrixFormat::kColwise;
520+
model.lp_.a_matrix_.start_ = a_start;
521+
model.lp_.a_matrix_.index_ = a_index;
522+
model.lp_.a_matrix_.value_ = a_value;
523+
524+
// Pass the model to HiGHS.
525+
HighsStatus status = highs.passModel(model);
526+
if (status != HighsStatus::kOk) {
527+
std::cerr << "Error passing LP model to HiGHS" << std::endl;
528+
}
529+
530+
// Set solver options
531+
highs.setOptionValue("primal_feasibility_tolerance", 1e-7);
532+
highs.setOptionValue("dual_feasibility_tolerance", 1e-7);
533+
highs.setOptionValue("simplex_iteration_limit", 10000);
534+
535+
// Perform initial solve
536+
highs.run();
545537

546538
this->is_initialized = true;
547-
}
539+
}
540+
541+
548542

549543
void dFBAModel::initModel(const char* sbmlFileName)
550544
{
@@ -555,61 +549,50 @@ void dFBAModel::initModel(const char* sbmlFileName)
555549

556550
void dFBAModel::writeProblem(const char *filename)
557551
{
558-
this->problem.writeLp(filename);
552+
highs.writeModel(filename); // if such a function exists
559553
}
560554

561555
dFBASolution dFBAModel::optimize()
562556
{
563-
564-
this->problem.dual();
565-
/* // If Dual fails, try Primal (more robust from scratch) --> if numerical issue occurs, try primal to make sure the cell is really dead
566-
if (this->problem.status() != 0) {
567-
this->problem.primal();
568-
} */
569-
bool isOptimal = problem.isProvenOptimal();
557+
// Solve the model using HiGHS
558+
HighsStatus run_status = highs.run();
570559

571560
solution.fluxes.clear();
572561

562+
// Get model status
563+
HighsModelStatus model_status = highs.getModelStatus();
573564
std::string status;
574-
switch (this->problem.status()) {
575-
case 0:
576-
status = "optimal";
577-
break;
578-
case 1:
579-
case 4:
580-
status = "infeasible";
581-
break;
582-
default:
583-
status = "unknown";
584-
std::cerr << "Solver returned unknown status code: " << this->problem.status() << std::endl;
585-
break;
565+
bool isOptimal = (model_status == HighsModelStatus::kOptimal);
566+
567+
if (isOptimal) {
568+
status = "optimal";
569+
} else if (model_status == HighsModelStatus::kInfeasible) {
570+
status = "infeasible";
571+
} else {
572+
status = "unknown";
573+
std::cerr << "Solver returned status: " << highs.modelStatusToString(model_status) << std::endl;
586574
}
587575

588576
if (isOptimal)
589577
{
590-
const double *columnPrimal = this->problem.getColSolution();
578+
const HighsSolution& sol = highs.getSolution();
579+
double fopt = highs.getInfo().objective_function_value;
591580

592-
double fopt = problem.getObjValue();
593-
594-
for(dFBAReaction* reaction: this->reactions)
581+
for (dFBAReaction* reaction : this->reactions)
595582
{
596583
int column_idx = this->reactionsIndexer[reaction->getId()];
597-
double flux = columnPrimal[column_idx];
584+
double flux = sol.col_value[column_idx];
598585
solution.fluxes[reaction->getId()] = flux;
599586
reaction->setFluxValue(flux);
600587
}
601588

602589
solution.objective_value = fopt;
603590
solution.status = status;
604-
605-
// Debugging info
606-
//std::cout << "Optimal solution found: Objective = " << fopt << "\n";
607591
}
608592
else if (status == "infeasible")
609593
{
610594
solution.status = status;
611595

612-
//std::cerr << "FBA optimization infeasible. Cell should die.\n";
613596
for (auto &reaction : this->reactions)
614597
{
615598
reaction->setFluxValue(0.0);
@@ -624,7 +607,6 @@ dFBASolution dFBAModel::optimize()
624607
std::cerr << "Reaction bounds at failure:\n";
625608
for (auto &reaction : this->reactions)
626609
{
627-
int idx = this->reactionsIndexer[reaction->getId()];
628610
double lb = reaction->getLowerBound();
629611
double ub = reaction->getUpperBound();
630612
std::cerr << reaction->getId() << ": [" << lb << ", " << ub << "]\n";
@@ -638,24 +620,23 @@ dFBASolution dFBAModel::optimize()
638620

639621
bool dFBAModel::getSolutionStatus()
640622
{
641-
if (this->is_initialized)
642-
return this->problem.isProvenOptimal();
643-
else
644-
return false;
623+
if (!this->is_initialized) return false;
624+
HighsModelStatus status = highs.getModelStatus();
625+
return (status == HighsModelStatus::kOptimal);
645626
}
646627

647628
double dFBAModel::getObjectiveValue()
648629
{
649630
assert(this->is_initialized);
650-
if (this->problem.isProvenOptimal())
651-
return this->problem.getObjValue();
652-
else
653-
std::cout << "WARNING: Primal infeasible" << std::endl;
654-
return 0;
631+
HighsModelStatus modelStatus = highs.getModelStatus();
632+
if (modelStatus == HighsModelStatus::kOptimal) {
633+
return highs.getInfo().objective_function_value;
634+
} else {
635+
std::cout << "WARNING: Primal infeasible or model not optimal" << std::endl;
636+
return 0;
637+
}
655638
}
656639

657640
bool dFBAModel::isInitialized(){
658641
return this->is_initialized;
659-
}
660-
661-
642+
}

0 commit comments

Comments
 (0)